From f5c99e9b2ee41ccae068516109c0e0c97f2eceab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dino=20Pa=C4=8Dandi?= <3002868+Dinonard@users.noreply.github.com> Date: Mon, 2 Sep 2024 17:42:00 +0200 Subject: [PATCH] Remove unused extrinsic from dapp staking (#1348) --- pallets/dapp-staking/src/lib.rs | 46 ---------------- pallets/dapp-staking/src/test/tests.rs | 72 -------------------------- pallets/dapp-staking/src/types.rs | 5 -- 3 files changed, 123 deletions(-) diff --git a/pallets/dapp-staking/src/lib.rs b/pallets/dapp-staking/src/lib.rs index a8dc283af8..c82b163d29 100644 --- a/pallets/dapp-staking/src/lib.rs +++ b/pallets/dapp-staking/src/lib.rs @@ -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. @@ -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, - account: T::AccountId, - ) -> DispatchResultWithPostInfo { - Self::ensure_pallet_enabled()?; - ensure_signed(origin)?; - - let mut ledger = Ledger::::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::::block_number().saturated_into(); - ledger - .unlocking - .iter_mut() - .for_each(|chunk| chunk.unlock_block = current_block); - Ledger::::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::::AccountNotInconsistent.into()) - } - } } impl Pallet { diff --git a/pallets/dapp-staking/src/test/tests.rs b/pallets/dapp-staking/src/test/tests.rs index 28e8636335..c3ffcbe732 100644 --- a/pallets/dapp-staking/src/test/tests.rs +++ b/pallets/dapp-staking/src/test/tests.rs @@ -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::::AccountNotInconsistent - ); - - assert_unlock(account_1, lock_1); - assert_noop!( - DappStaking::fix_account(RuntimeOrigin::signed(11), account_1), - Error::::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::::ZeroAmount, - ); - - // Now reproduce the incorrect lock/freeze operation. - let mut ledger = Ledger::::get(&account_2); - ledger.add_lock_amount(unlock_2); - assert_ok!(DappStaking::update_ledger(&account_2, ledger)); - use crate::CurrentEraInfo; - CurrentEraInfo::::mutate(|era_info| { - era_info.add_locked(unlock_2); - }); - assert!( - Balances::free_balance(&account_2) - < Ledger::::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::::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::::AccountNotInconsistent - ); - }) -} - #[test] fn claim_staker_rewards_for_basic_example_is_ok() { ExtBuilder::default().build_and_execute(|| { diff --git a/pallets/dapp-staking/src/types.rs b/pallets/dapp-staking/src/types.rs index d49720865a..8cad803274 100644 --- a/pallets/dapp-staking/src/types.rs +++ b/pallets/dapp-staking/src/types.rs @@ -1618,11 +1618,6 @@ pub struct TiersConfiguration, T: TierSlotsFunc, P: Get> 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, T: TierSlotsFunc, P: Get> TiersConfiguration { /// Check if parameters are valid. pub fn is_valid(&self) -> bool {