Skip to content

Commit

Permalink
Remove unused extrinsic from dapp staking (#1348)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dinonard
Dinonard authored Sep 2, 2024
1 parent 2d0aa10 commit f5c99e9
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 123 deletions.
46 changes: 0 additions & 46 deletions pallets/dapp-staking/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,6 @@ pub mod pallet {
NoExpiredEntries,
/// Force call is not allowed in production.
ForceNotAllowed,
/// Account doesn't have the freeze inconsistency
AccountNotInconsistent, // TODO: can be removed after call `fix_account` is removed
}

/// General information about dApp staking protocol state.
Expand Down Expand Up @@ -1520,50 +1518,6 @@ pub mod pallet {

Self::internal_claim_bonus_reward_for(account, smart_contract)
}

/// A call used to fix accounts with inconsistent state, where frozen balance is actually higher than what's available.
///
/// The approach is as simple as possible:
/// 1. Caller provides an account to fix.
/// 2. If account is eligible for the fix, all unlocking chunks are modified to be claimable immediately.
/// 3. The `claim_unlocked` call is executed using the provided account as the origin.
/// 4. All states are updated accordingly, and the account is no longer in an inconsistent state.
///
/// The benchmarked weight of the `claim_unlocked` call is used as a base, and additional overestimated weight is added.
/// Call doesn't touch any storage items that aren't already touched by the `claim_unlocked` call, hence the simplified approach.
#[pallet::call_index(100)]
#[pallet::weight(T::DbWeight::get().reads_writes(4, 1))]
pub fn fix_account(
origin: OriginFor<T>,
account: T::AccountId,
) -> DispatchResultWithPostInfo {
Self::ensure_pallet_enabled()?;
ensure_signed(origin)?;

let mut ledger = Ledger::<T>::get(&account);
let locked_amount_ledger = ledger.total_locked_amount();
let total_balance = T::Currency::total_balance(&account);

if locked_amount_ledger > total_balance {
// 1. Modify all unlocking chunks so they can be unlocked immediately.
let current_block: BlockNumber =
frame_system::Pallet::<T>::block_number().saturated_into();
ledger
.unlocking
.iter_mut()
.for_each(|chunk| chunk.unlock_block = current_block);
Ledger::<T>::insert(&account, ledger);

// 2. Execute the unlock call, clearing all of the unlocking chunks.
Self::internal_claim_unlocked(account)?;

// 3. In case of success, ensure no fee is paid.
Ok(Pays::No.into())
} else {
// The above logic is designed for a specific scenario and cannot be used otherwise.
Err(Error::<T>::AccountNotInconsistent.into())
}
}
}

impl<T: Config> Pallet<T> {
Expand Down
72 changes: 0 additions & 72 deletions pallets/dapp-staking/src/test/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3373,78 +3373,6 @@ fn lock_correctly_considers_unlocking_amount() {
})
}

#[test]
fn fix_account_scenarios_work() {
ExtBuilder::default().build_and_execute(|| {
// 1. Lock some amount correctly, unstake it, try to fix it, and ensure the call fails
let (account_1, lock_1) = (1, 100);
assert_lock(account_1, lock_1);
assert_noop!(
DappStaking::fix_account(RuntimeOrigin::signed(11), account_1),
Error::<Test>::AccountNotInconsistent
);

assert_unlock(account_1, lock_1);
assert_noop!(
DappStaking::fix_account(RuntimeOrigin::signed(11), account_1),
Error::<Test>::AccountNotInconsistent
);

// 2. Reproduce the issue where the account has more frozen than balance
let (account_2, unlock_2) = (2, 13);
let lock_2 = Balances::total_balance(&account_2);
assert_lock(account_2, lock_2);
assert_unlock(account_2, unlock_2);

// With the fix implemented, the scenario needs to be reproduced by hand.
// Account calls `lock`, specifying the amount that is undergoing the unlocking process.
// It can be either more or less, it doesn't matter for the test or the issue.

// But first, a sanity check.
assert_noop!(
DappStaking::lock(RuntimeOrigin::signed(account_2), unlock_2),
Error::<Test>::ZeroAmount,
);

// Now reproduce the incorrect lock/freeze operation.
let mut ledger = Ledger::<Test>::get(&account_2);
ledger.add_lock_amount(unlock_2);
assert_ok!(DappStaking::update_ledger(&account_2, ledger));
use crate::CurrentEraInfo;
CurrentEraInfo::<Test>::mutate(|era_info| {
era_info.add_locked(unlock_2);
});
assert!(
Balances::free_balance(&account_2)
< Ledger::<Test>::get(&account_2).total_locked_amount(),
"Sanity check."
);

// Now fix the account
assert_ok!(DappStaking::fix_account(
RuntimeOrigin::signed(11),
account_2
));
System::assert_last_event(RuntimeEvent::DappStaking(Event::ClaimedUnlocked {
account: account_2,
amount: unlock_2,
}));

// Post-fix checks
assert_eq!(
Balances::free_balance(&account_2),
Ledger::<Test>::get(&account_2).total_locked_amount(),
"After the fix, balances should be equal."
);

// Cannot fix the same account again.
assert_noop!(
DappStaking::fix_account(RuntimeOrigin::signed(11), account_2),
Error::<Test>::AccountNotInconsistent
);
})
}

#[test]
fn claim_staker_rewards_for_basic_example_is_ok() {
ExtBuilder::default().build_and_execute(|| {
Expand Down
5 changes: 0 additions & 5 deletions pallets/dapp-staking/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1618,11 +1618,6 @@ pub struct TiersConfiguration<NT: Get<u32>, T: TierSlotsFunc, P: Get<FixedU128>>
pub(crate) _phantom: PhantomData<(T, P)>,
}

// Some TODOs:
// Some parts regarding tiers should be refactored.
// * There's no need to keep thresholds in two separate storage items since the calculation can always be done compared to the
// anchor value of 5 cents. This still needs to be checked & investigated, but it's worth a try.

impl<NT: Get<u32>, T: TierSlotsFunc, P: Get<FixedU128>> TiersConfiguration<NT, T, P> {
/// Check if parameters are valid.
pub fn is_valid(&self) -> bool {
Expand Down

0 comments on commit f5c99e9

Please sign in to comment.