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.

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

refactor: Replace MinimumStake with MinimumTotalStake in creator-staking pallet #253

Merged
merged 8 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 26 additions & 24 deletions pallets/creator-staking/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,22 @@ impl<T: Config> Pallet<T> {
/// If the stake operation was successful, the given structs are properly modified.
/// If not, an error is returned and the structs are left in an undefined state.
pub(crate) fn stake_to_creator(
backer: &T::AccountId,
backer_stakes: &mut StakesInfoOf<T>,
staking_info: &mut CreatorStakeInfo<BalanceOf<T>>,
creator_stake_info: &mut CreatorStakeInfo<BalanceOf<T>>,
amount: BalanceOf<T>,
current_era: EraIndex,
) -> Result<(), DispatchError> {
let current_stake = backer_stakes.current_stake();
let current_era = Self::current_era();
let staked_before = backer_stakes.current_stake();

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

backer_stakes
Expand All @@ -119,21 +120,19 @@ impl<T: Config> Pallet<T> {

Self::ensure_can_add_stake_item(backer_stakes)?;

ensure!(
backer_stakes.current_stake() >= T::MinimumStake::get(),
Error::<T>::InsufficientStakingAmount,
);
let total_stake = Self::total_staked_amount(&backer).saturating_add(amount);
ensure!(total_stake >= T::MinimumTotalStake::get(), Error::<T>::InsufficientStakingAmount);

// Increment the backer's total deposit for a particular creator.
staking_info.total_staked = staking_info.total_staked.saturating_add(amount);
creator_stake_info.total_staked = creator_stake_info.total_staked.saturating_add(amount);

Ok(())
}

/// A utility method used to unstake a specified amount from an arbitrary creator.
///
/// The amount unstaked can be different in case the staked amount would fall bellow
/// `MinimumStake`. In that case, the entire staked amount will be unstaked.
/// The amount unstaked can be different in case the staked amount would fall bellow zero.
/// In that case, only the staked amount will be unstaked.
///
/// `StakesInfoOf` and `CreatorStakeInfo` are provided and all checks are made to ensure that
/// it's possible to complete the unstake operation.
Expand All @@ -152,18 +151,16 @@ impl<T: Config> Pallet<T> {
/// an undefined state.
pub(crate) fn calculate_and_apply_stake_decrease(
backer_stakes: &mut StakesInfoOf<T>,
stake_info: &mut CreatorStakeInfo<BalanceOf<T>>,
creator_stake_info: &mut CreatorStakeInfo<BalanceOf<T>>,
desired_amount: BalanceOf<T>,
current_era: EraIndex,
) -> Result<BalanceOf<T>, DispatchError> {
let current_era = Self::current_era();
let staked_value = backer_stakes.current_stake();
ensure!(staked_value > Zero::zero(), Error::<T>::NotStakedCreator);

let remaining = staked_value.saturating_sub(desired_amount);

// If the remaining amount is less than the minimum staking amount, unstake the entire amount.
let amount_to_decrease = if remaining < T::MinimumStake::get() {
stake_info.backers_count = stake_info.backers_count.saturating_sub(1);
// If the remaining amount equals to zero, unstake the entire amount by this creator.
let amount_to_decrease = if desired_amount >= staked_value {
creator_stake_info.backers_count = creator_stake_info.backers_count.saturating_sub(1);
staked_value
} else {
desired_amount
Expand All @@ -173,7 +170,7 @@ impl<T: Config> Pallet<T> {
ensure!(amount_to_decrease > Zero::zero(), Error::<T>::CannotUnstakeZero);

// Modify data
stake_info.total_staked = stake_info.total_staked.saturating_sub(amount_to_decrease);
creator_stake_info.total_staked = creator_stake_info.total_staked.saturating_sub(amount_to_decrease);

backer_stakes
.decrease_stake(current_era, amount_to_decrease)
Expand All @@ -189,9 +186,9 @@ impl<T: Config> Pallet<T> {
pub(crate) fn update_backer_locks(backer: &T::AccountId, backer_locks: BackerLocksOf<T>) {
if backer_locks.is_empty() {
BackerLocksByAccount::<T>::remove(backer);
T::Currency::remove_lock(STAKING_ID, backer);
T::Currency::remove_lock(STAKING_LOCK_ID, backer);
} else {
T::Currency::set_lock(STAKING_ID, backer, backer_locks.total_locked, WithdrawReasons::all());
T::Currency::set_lock(STAKING_LOCK_ID, backer, backer_locks.total_locked, WithdrawReasons::all());
BackerLocksByAccount::<T>::insert(backer, backer_locks);
}
}
Expand Down Expand Up @@ -227,6 +224,11 @@ impl<T: Config> Pallet<T> {
T::PalletId::get().into_account_truncating()
}

pub(crate) fn total_staked_amount(backer: &T::AccountId) -> BalanceOf<T> {
let backer_locks = BackerLocksByAccount::<T>::get(&backer);
backer_locks.total_staked()
}

/// The block rewards are accumulated in this pallet's account during each era.
/// This function takes a snapshot of the pallet's balance accrued during the current era
/// and stores it for future distribution.
Expand Down
27 changes: 16 additions & 11 deletions pallets/creator-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub mod pallet {

/// An identifier for the locks made in this pallet.
/// Used to determine the locks in this pallet so that they can be replaced or removed.
pub(crate) const STAKING_ID: LockIdentifier = *b"crestake";
pub(crate) const STAKING_LOCK_ID: LockIdentifier = *b"crestake";

#[pallet::config]
pub trait Config: frame_system::Config + pallet_permissions::Config {
Expand Down Expand Up @@ -68,11 +68,10 @@ pub mod pallet {
#[pallet::constant]
type CreatorRegistrationDeposit: Get<BalanceOf<Self>>;

/// The minimum amount that can be staked to the creator.
/// User can stake less if they already have the minimum staking amount staked to that
/// particular creator.
/// The minimum amount that should be staked in the creator staking system.
/// User can stake less if they already have the minimum stake staked in the system.
#[pallet::constant]
type MinimumStake: Get<BalanceOf<Self>>;
type MinimumTotalStake: Get<BalanceOf<Self>>;

// TODO: make it MinimumRemainingRatio
// (e.g. 0.1 = 10%, so that account can lock only 90% of its balance)
Expand Down Expand Up @@ -409,10 +408,10 @@ pub mod pallet {
Self::creator_stake_info(creator_id, current_era).unwrap_or_default();

Self::stake_to_creator(
&backer,
&mut backer_stakes,
&mut staking_info,
amount_to_stake,
current_era,
)?;

backer_locks.total_locked = backer_locks.total_locked.saturating_add(amount_to_stake);
Expand Down Expand Up @@ -461,14 +460,21 @@ pub mod pallet {

let current_era = Self::current_era();
let mut backer_stakes = Self::backer_stakes(&backer, creator_id);
let mut stake_info =
let mut creator_stake_info =
Self::creator_stake_info(creator_id, current_era).unwrap_or_default();

let amount_to_unstake =
Self::calculate_and_apply_stake_decrease(&mut backer_stakes, &mut stake_info, amount, current_era)?;
Self::calculate_and_apply_stake_decrease(&mut backer_stakes, &mut creator_stake_info, amount)?;

// Update the chunks and write them to storage
let mut backer_locks = Self::backer_locks(&backer);

let total_stake_remaining = backer_locks.total_staked().saturating_sub(amount_to_unstake);
ensure!(
total_stake_remaining.is_zero() || total_stake_remaining >= T::MinimumTotalStake::get(),
Error::<T>::InsufficientStakingAmount,
);

backer_locks.unbonding_info.add(UnbondingChunk {
amount: amount_to_unstake,
unlock_era: current_era + T::UnbondingPeriodInEras::get(),
Expand All @@ -483,7 +489,7 @@ pub mod pallet {
}
});
Self::update_backer_stakes(&backer, creator_id, backer_stakes);
CreatorStakeInfoByEra::<T>::insert(creator_id, current_era, stake_info);
CreatorStakeInfoByEra::<T>::insert(creator_id, current_era, creator_stake_info);

Self::deposit_event(Event::<T>::Unstaked {
who: backer,
Expand Down Expand Up @@ -624,7 +630,6 @@ pub mod pallet {
&mut backer_stakes_by_source_creator,
&mut source_creator_info,
amount,
current_era,
)?;

// Validate and update target creator related data
Expand All @@ -633,10 +638,10 @@ pub mod pallet {
Self::creator_stake_info(to_creator_id, current_era).unwrap_or_default();

Self::stake_to_creator(
&backer,
&mut backer_stakes_by_target_creator,
&mut target_creator_info,
stake_amount_to_move,
current_era,
)?;

CreatorStakeInfoByEra::<T>::insert(from_creator_id, current_era, source_creator_info);
Expand Down
6 changes: 3 additions & 3 deletions pallets/creator-staking/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(super) const EXISTENTIAL_DEPOSIT: Balance = 2;
pub(super) const REGISTER_DEPOSIT: Balance = 10;
pub(super) const MAX_NUMBER_OF_BACKERS: u32 = 4;
/// Value shouldn't be less than 2 for testing purposes, otherwise we cannot test certain corner cases.
pub(super) const MINIMUM_STAKING_AMOUNT: Balance = 10;
pub(super) const MINIMUM_TOTAL_STAKE: Balance = 10;
pub(super) const MINIMUM_REMAINING_AMOUNT: Balance = 1;
pub(super) const MAX_UNBONDING_CHUNKS: u32 = 5;
pub(super) const UNBONDING_PERIOD_IN_ERAS: EraIndex = 3;
Expand Down Expand Up @@ -137,7 +137,7 @@ parameter_types! {
pub const CreatorRegistrationDeposit: Balance = REGISTER_DEPOSIT;
pub const BlockPerEra: BlockNumber = BLOCKS_PER_ERA;
pub const MaxNumberOfBackersPerCreator: u32 = MAX_NUMBER_OF_BACKERS;
pub const MinimumStake: Balance = MINIMUM_STAKING_AMOUNT;
pub const MinimumTotalStake: Balance = MINIMUM_TOTAL_STAKE;
pub const CreatorStakingPalletId: PalletId = PalletId(*b"mokcrstk");
pub const MinimumRemainingFreeBalance: Balance = MINIMUM_REMAINING_AMOUNT;
#[derive(PartialEq)]
Expand Down Expand Up @@ -185,7 +185,7 @@ impl pallet_creator_staking::Config for TestRuntime {
type SpacesInterface = MockSpaces;
type SpacePermissionsProvider = MockSpaces;
type CreatorRegistrationDeposit = CreatorRegistrationDeposit;
type MinimumStake = MinimumStake;
type MinimumTotalStake = MinimumTotalStake;
type MinimumRemainingFreeBalance = MinimumRemainingFreeBalance;
type MaxNumberOfBackersPerCreator = MaxNumberOfBackersPerCreator;
type MaxEraStakeItems = MaxEraStakeItems;
Expand Down
6 changes: 3 additions & 3 deletions pallets/creator-staking/src/tests/testing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ pub(super) fn assert_unstake(
.backer_stakes
.current_stake()
.saturating_sub(value);
let expected_unbond_amount = if remaining_staked < MINIMUM_STAKING_AMOUNT {
let expected_unbond_amount = if remaining_staked < MINIMUM_TOTAL_STAKE {
init_state.backer_stakes.current_stake()
} else {
value
Expand Down Expand Up @@ -610,9 +610,9 @@ pub(crate) fn assert_move_stake(
let source_creator_init_state = MemorySnapshot::all(current_era, from_creator_id, backer);
let target_creator_init_state = MemorySnapshot::all(current_era, to_creator_id, backer);

// Calculate value which will actually be transfered
// Calculate value which will actually be transferred
let source_creator_init_stake_amount = source_creator_init_state.backer_stakes.current_stake();
let expected_amount_to_move = if source_creator_init_stake_amount - amount >= MINIMUM_STAKING_AMOUNT {
let expected_amount_to_move = if amount < source_creator_init_stake_amount {
amount
} else {
source_creator_init_stake_amount
Expand Down
55 changes: 13 additions & 42 deletions pallets/creator-staking/src/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ fn stake_insufficient_value() {
CreatorStaking::stake(
RuntimeOrigin::signed(backer_id),
creator_id.clone(),
MINIMUM_STAKING_AMOUNT - 1
MINIMUM_TOTAL_STAKE - 1
),
Error::<TestRuntime>::InsufficientStakingAmount
);
Expand Down Expand Up @@ -860,7 +860,7 @@ fn unstake_multiple_time_is_ok() {
let stakeholder = 10;
let backer_id = 1;
let creator_id = 1;
let original_staked_value = 300 + MINIMUM_STAKING_AMOUNT;
let original_staked_value = 300 + MINIMUM_TOTAL_STAKE;
let old_era = CreatorStaking::current_era();

// Insert a creator under registered creators, stake it.
Expand All @@ -880,25 +880,24 @@ fn unstake_multiple_time_is_ok() {
}

#[test]
fn unstake_value_below_staking_threshold() {
fn unstake_value_below_staked_to_creator() {
ExternalityBuilder::build().execute_with(|| {
initialize_first_block();

let stakeholder = 10;
let backer_id = 1;
let creator_id = 1;
let first_value_to_unstake = 300;
let staked_value = first_value_to_unstake + MINIMUM_STAKING_AMOUNT;
let staked_value = MINIMUM_TOTAL_STAKE * 2;

// Insert a creator under registered creators, stake it.
assert_register(stakeholder, creator_id);
assert_stake(backer_id, creator_id, staked_value);

// Unstake such an amount that exactly minimum staking amount will remain staked.
assert_unstake(backer_id, creator_id, first_value_to_unstake);
assert_unstake(backer_id, creator_id, MINIMUM_TOTAL_STAKE);

// Unstake 1 token and expect that the entire staked amount will be unstaked.
assert_unstake(backer_id, creator_id, 1);
// Unstake 2x of remaining tokens and expect that the entire staked amount will be unstaked.
assert_unstake(backer_id, creator_id, MINIMUM_TOTAL_STAKE * 2);
})
}

Expand Down Expand Up @@ -985,7 +984,7 @@ fn unstake_too_many_unbonding_chunks_is_not_ok() {
let backer = 1;
let unstake_amount = 10;
let stake_amount =
MINIMUM_STAKING_AMOUNT * 10 + unstake_amount * MAX_UNBONDING_CHUNKS as Balance;
MINIMUM_TOTAL_STAKE * 10 + unstake_amount * MAX_UNBONDING_CHUNKS as Balance;

assert_stake(backer, creator_id, stake_amount);

Expand Down Expand Up @@ -1846,14 +1845,14 @@ fn move_stake_is_ok() {

assert_register(stakeholder, from_creator_id);
assert_register(stakeholder, to_creator_id);
assert_stake(backer, from_creator_id, MINIMUM_STAKING_AMOUNT * 2);
assert_stake(backer, from_creator_id, MINIMUM_TOTAL_STAKE * 2);

// The first transfer will ensure that both creators are staked after operation is complete
assert_move_stake(
backer,
from_creator_id,
to_creator_id,
MINIMUM_STAKING_AMOUNT,
MINIMUM_TOTAL_STAKE,
);
assert!(
!BackerStakesByCreator::<TestRuntime>::get(&backer, &from_creator_id)
Expand All @@ -1866,7 +1865,7 @@ fn move_stake_is_ok() {
backer,
from_creator_id,
to_creator_id,
MINIMUM_STAKING_AMOUNT,
MINIMUM_TOTAL_STAKE,
);
assert!(
BackerStakesByCreator::<TestRuntime>::get(&backer, &from_creator_id)
Expand Down Expand Up @@ -1908,7 +1907,7 @@ fn move_stake_should_work_from_inactive_creator() {
let source_creator_id = 1;
let target_creator_id = 2;

let stake_amount = MINIMUM_STAKING_AMOUNT * 2;
let stake_amount = MINIMUM_TOTAL_STAKE * 2;

assert_register(stakeholder, source_creator_id);
assert_register(stakeholder, target_creator_id);
Expand All @@ -1930,7 +1929,7 @@ fn move_stake_to_inactive_creator_should_fail() {
let source_creator_id = 1;
let target_creator_id = 2;

let stake_amount = MINIMUM_STAKING_AMOUNT * 2;
let stake_amount = MINIMUM_TOTAL_STAKE * 2;

assert_register(stakeholder, source_creator_id);
assert_register(stakeholder, target_creator_id);
Expand Down Expand Up @@ -2001,34 +2000,6 @@ fn move_zero_stake_should_fail() {
})
}

#[test]
// When target_creator wasn't staked by a backer before,
// moving amount should be more than MinimumStakingAmount
fn move_stake_should_fail_with_insufficient_staking_amount() {
ExternalityBuilder::build().execute_with(|| {
initialize_first_block();

let stakeholder = 1;
let backer = 3;
let source_creator_id = 1;
let target_creator_id = 2;

assert_register(stakeholder, source_creator_id);
assert_register(stakeholder, target_creator_id);
assert_stake(backer, source_creator_id, 100);

assert_noop!(
CreatorStaking::move_stake(
RuntimeOrigin::signed(backer),
source_creator_id,
target_creator_id,
MINIMUM_STAKING_AMOUNT - 1,
),
Error::<TestRuntime>::InsufficientStakingAmount
);
})
}

#[test]
fn move_stake_with_max_era_stake_items_exceeded_should_fail() {
ExternalityBuilder::build().execute_with(|| {
Expand Down
4 changes: 4 additions & 0 deletions pallets/creator-staking/src/types/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ impl<Balance, MaxUnbondingChunks> BackerLocks<Balance, MaxUnbondingChunks>
pub fn is_empty(&self) -> bool {
self.total_locked.is_zero() && self.unbonding_info.is_empty()
}

pub fn total_staked(&self) -> Balance {
self.total_locked.saturating_sub(self.unbonding_info.sum())
}
}

impl<Balance: AtLeast32BitUnsigned + Copy + MaxEncodedLen> EraStake<Balance> {
Expand Down
Loading
Loading