Skip to content

Commit

Permalink
Refactoring & cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dinonard
Dinonard committed Oct 4, 2023
1 parent c3834fb commit a790548
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 576 deletions.
39 changes: 12 additions & 27 deletions pallets/dapp-staking-v3/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,6 @@ pub mod pallet {
Self::ensure_pallet_enabled()?;
let account = ensure_signed(origin)?;

let state = ActiveProtocolState::<T>::get();
let mut ledger = Ledger::<T>::get(&account);

// Calculate & check amount available for locking
Expand All @@ -464,11 +463,8 @@ pub mod pallet {
let amount_to_lock = available_balance.min(amount);
ensure!(!amount_to_lock.is_zero(), Error::<T>::ZeroAmount);

// Only lock for the next era onwards.
let lock_era = state.era.saturating_add(1);
ledger
.add_lock_amount(amount_to_lock, lock_era)
.map_err(|_| Error::<T>::TooManyLockedBalanceChunks)?;
ledger.add_lock_amount(amount_to_lock);

ensure!(
ledger.active_locked_amount() >= T::MinimumLockedAmount::get(),
Error::<T>::LockedAmountBelowThreshold
Expand Down Expand Up @@ -522,9 +518,7 @@ pub mod pallet {
ensure!(!amount_to_unlock.is_zero(), Error::<T>::ZeroAmount);

// Update ledger with new lock and unlocking amounts
ledger
.subtract_lock_amount(amount_to_unlock, state.era)
.map_err(|_| Error::<T>::TooManyLockedBalanceChunks)?;
ledger.subtract_lock_amount(amount_to_unlock);

let current_block = frame_system::Pallet::<T>::block_number();
let unlock_block = current_block.saturating_add(T::UnlockingPeriod::get());
Expand Down Expand Up @@ -577,18 +571,13 @@ pub mod pallet {
Self::ensure_pallet_enabled()?;
let account = ensure_signed(origin)?;

let state = ActiveProtocolState::<T>::get();
let mut ledger = Ledger::<T>::get(&account);

ensure!(!ledger.unlocking.is_empty(), Error::<T>::NoUnlockingChunks);

// Only lock for the next era onwards.
let lock_era = state.era.saturating_add(1);
let amount = ledger.consume_unlocking_chunks();

ledger
.add_lock_amount(amount, lock_era)
.map_err(|_| Error::<T>::TooManyLockedBalanceChunks)?;
ledger.add_lock_amount(amount);
ensure!(
ledger.active_locked_amount() >= T::MinimumLockedAmount::get(),
Error::<T>::LockedAmountBelowThreshold
Expand Down Expand Up @@ -626,15 +615,14 @@ pub mod pallet {

let protocol_state = ActiveProtocolState::<T>::get();
let mut ledger = Ledger::<T>::get(&account);
let stake_era = protocol_state.era.saturating_add(1);

// TODO: staker should be staking from the NEXT era

// 1.
// Increase stake amount for the current era & period in staker's ledger
// Increase stake amount for the next era & current period in staker's ledger
ledger
.add_stake_amount(
amount,
protocol_state.era,
protocol_state.period_info.number,
)
.add_stake_amount(amount, stake_era, protocol_state.period_info.number)
.map_err(|err| match err {
AccountLedgerError::InvalidPeriod => {
Error::<T>::UnclaimedRewardsFromPastPeriods
Expand All @@ -646,10 +634,9 @@ pub mod pallet {

// 2.
// Update `StakerInfo` storage with the new stake amount on the specified contract.
// TODO: this might need to be modified - if rewards were claimed for the past period, it should be ok to overwrite the old storage item.
let new_staking_info =
if let Some(mut staking_info) = StakerInfo::<T>::get(&account, &smart_contract) {
// TODO: do I need to check for which period this is for? Not sure, but maybe it's safer to do so.
// TODO2: revisit this later.
ensure!(
staking_info.period_number() == protocol_state.period_info.number,
Error::<T>::InternalStakeError
Expand All @@ -674,7 +661,7 @@ pub mod pallet {
let mut contract_stake_info = ContractStake::<T>::get(&smart_contract);
ensure!(
contract_stake_info
.stake(amount, protocol_state.period_info, protocol_state.era)
.stake(amount, protocol_state.period_info, stake_era)
.is_ok(),
Error::<T>::InternalStakeError
);
Expand All @@ -700,9 +687,7 @@ pub mod pallet {
// TODO: maybe keep track of pending bonus rewards in the AccountLedger struct?
// That way it's easy to check if stake can even be called - bonus-rewards should be zero & last staked era should be None or current one.

// is it voting or b&e period?
// how much does user have available for staking?
// has user claimed past rewards? Can we force them to do it before they start staking again?
// TODO: has user claimed past rewards? Can we force them to do it before they start staking again?

Ok(())
}
Expand Down
12 changes: 0 additions & 12 deletions pallets/dapp-staking-v3/src/test/testing_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,6 @@ pub(crate) fn assert_lock(account: AccountId, amount: Balance) {
locked_balance + expected_lock_amount,
"Locked balance should be increased by the amount locked."
);
assert_eq!(
post_snapshot
.ledger
.get(&account)
.expect("Ledger entry has to exist after succcessful lock call")
.lock_era(),
post_snapshot.active_protocol_state.era + 1
);

assert_eq!(
post_snapshot.current_era_info.total_locked,
Expand Down Expand Up @@ -382,10 +374,6 @@ pub(crate) fn assert_relock_unlocking(account: AccountId) {
post_ledger.active_locked_amount(),
pre_snapshot.ledger[&account].active_locked_amount() + amount
);
assert_eq!(
post_ledger.lock_era(),
post_snapshot.active_protocol_state.era + 1
);

// Current era info
assert_eq!(
Expand Down
76 changes: 0 additions & 76 deletions pallets/dapp-staking-v3/src/test/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,35 +355,6 @@ fn lock_with_incorrect_amount_fails() {
})
}

#[test]
fn lock_with_too_many_chunks_fails() {
ExtBuilder::build().execute_with(|| {
let max_locked_chunks = <Test as pallet_dapp_staking::Config>::MaxLockedChunks::get();
let minimum_locked_amount: Balance =
<Test as pallet_dapp_staking::Config>::MinimumLockedAmount::get();

// Fill up the locked chunks to the limit
let locker = 1;
assert_lock(locker, minimum_locked_amount);
for current_era in 1..max_locked_chunks {
advance_to_era(current_era + 1);
assert_lock(locker, 1);
}

// Ensure we can still lock in the current era since number of chunks should not increase
for _ in 0..10 {
assert_lock(locker, 1);
}

// Advance to the next era and ensure it's not possible to add additional chunks
advance_to_era(ActiveProtocolState::<Test>::get().era + 1);
assert_noop!(
DappStaking::lock(RuntimeOrigin::signed(locker), 1),
Error::<Test>::TooManyLockedBalanceChunks,
);
})
}

#[test]
fn unlock_basic_example_is_ok() {
ExtBuilder::build().execute_with(|| {
Expand Down Expand Up @@ -558,33 +529,6 @@ fn unlock_with_zero_amount_fails() {
})
}

#[test]
fn unlock_with_exceeding_locked_storage_limits_fails() {
ExtBuilder::build().execute_with(|| {
let account = 2;
let lock_amount = 103;
assert_lock(account, lock_amount);

let unlock_amount = 3;
for _ in 0..<Test as pallet_dapp_staking::Config>::MaxLockedChunks::get() {
advance_to_era(ActiveProtocolState::<Test>::get().era + 1);
assert_unlock(account, unlock_amount);
}

// We can still unlock in the current era, theoretically
for _ in 0..5 {
assert_unlock(account, unlock_amount);
}

// Following unlock should fail due to exceeding storage limits
advance_to_era(ActiveProtocolState::<Test>::get().era + 1);
assert_noop!(
DappStaking::unlock(RuntimeOrigin::signed(account), unlock_amount),
Error::<Test>::TooManyLockedBalanceChunks,
);
})
}

#[test]
fn unlock_with_exceeding_unlocking_chunks_storage_limits_fails() {
ExtBuilder::build().execute_with(|| {
Expand Down Expand Up @@ -709,26 +653,6 @@ fn relock_unlocking_no_chunks_fails() {
})
}

#[test]
fn relock_unlocking_too_many_chunks_fails() {
ExtBuilder::build().execute_with(|| {
let max_locked_chunks = <Test as pallet_dapp_staking::Config>::MaxLockedChunks::get();

// Fill up the locked chunks to the limit
let account = 3;
for current_era in 1..=max_locked_chunks {
assert_lock(account, 11);
advance_to_era(current_era + 1);
}
assert_unlock(account, 7);

assert_noop!(
DappStaking::relock_unlocking(RuntimeOrigin::signed(account)),
Error::<Test>::TooManyLockedBalanceChunks,
);
})
}

#[test]
fn relock_unlocking_insufficient_lock_amount_fails() {
ExtBuilder::build().execute_with(|| {
Expand Down
Loading

0 comments on commit a790548

Please sign in to comment.