--- a/Cargo.lock +++ b/Cargo.lock @@ -5803,11 +5803,12 @@ [[package]] name = "pallet-app-promotion" -version = "0.1.3" +version = "0.1.4" dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log", "pallet-balances", "pallet-common", "pallet-configuration", --- a/pallets/app-promotion/CHANGELOG.md +++ b/pallets/app-promotion/CHANGELOG.md @@ -4,6 +4,13 @@ +## [0.1.4] - 2023-01-31 + +### Changed + +- Balance reservation when calling the unstake method is removed. + Now the locked balance (by AppPromo) does not change until the unlock interval expires. + ## [0.1.3] - 2022-12-25 ### Fixed --- a/pallets/app-promotion/Cargo.toml +++ b/pallets/app-promotion/Cargo.toml @@ -9,7 +9,7 @@ license = 'GPLv3' name = 'pallet-app-promotion' repository = 'https://github.com/UniqueNetwork/unique-chain' -version = '0.1.3' +version = '0.1.4' [package.metadata.docs.rs] targets = ['x86_64-unknown-linux-gnu'] @@ -67,3 +67,6 @@ # [dev-dependencies] ################################################################################ +# Other + +log = { version = "0.4.16", default-features = false } \ No newline at end of file --- a/pallets/app-promotion/src/lib.rs +++ b/pallets/app-promotion/src/lib.rs @@ -97,7 +97,7 @@ use super::*; use frame_support::{ Blake2_128Concat, Twox64Concat, pallet_prelude::*, storage::Key, PalletId, - traits::ReservableCurrency, + traits::ReservableCurrency, weights::Weight, }; use frame_system::pallet_prelude::*; @@ -234,7 +234,7 @@ QueryKind = ValueQuery, >; - /// Stores amount of stakes for an `Account`. + /// Stores number of stake records for an `Account`. /// /// * **Key** - Staker account. /// * **Value** - Amount of stakes. @@ -242,7 +242,7 @@ pub type StakesPerAccount = StorageMap<_, Blake2_128Concat, T::AccountId, u8, ValueQuery>; - /// Stores amount of stakes for an `Account`. + /// Pending unstake records for an `Account`. /// /// * **Key** - Staker account. /// * **Value** - Amount of stakes. @@ -262,6 +262,10 @@ pub type PreviousCalculatedRecord = StorageValue; + #[pallet::storage] + pub(crate) type UpgradedToReserves = + StorageValue; + #[pallet::hooks] impl Hooks> for Pallet { /// Block overflow is impossible due to the fact that the unstake algorithm in on_initialize @@ -276,13 +280,104 @@ if !block_pending.is_empty() { block_pending.into_iter().for_each(|(staker, amount)| { + Self::get_locked_balance(&staker).map(|b| { + let new_state = b.amount.checked_sub(&amount).unwrap_or_default(); + Self::set_lock_unchecked(&staker, new_state); + }); + }); + } + + ::WeightInfo::on_initialize(counter) + } + + fn on_runtime_upgrade() -> Weight { + let mut consumed_weight = Weight::zero(); + let mut add_weight = |reads, writes, weight| { + consumed_weight += T::DbWeight::get().reads_writes(reads, writes); + consumed_weight += weight; + }; + + if >::get() { + add_weight(1, 0, Weight::zero()); + return consumed_weight; + } else { + add_weight(1, 1, Weight::zero()); + >::set(true); + } + >::drain().for_each(|(_, v)| { + add_weight(1, 1, Weight::zero()); + v.into_iter().for_each(|(staker, amount)| { <::Currency as ReservableCurrency>::unreserve( &staker, amount, ); + add_weight(1, 1, Weight::zero()); }); + }); + + consumed_weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + use sp_std::collections::btree_map::BTreeMap; + if >::get() { + return Ok(Default::default()); + } + // Staker -> (total amount of reserved balance, reserved by promotion); + let mut pre_state: BTreeMap, BalanceOf)> = + BTreeMap::new(); + + >::iter().for_each(|(_, v)| { + v.into_iter().for_each(|(staker, amount)| { + if let Some((_, reserved_balance)) = pre_state.get_mut(&staker) { + *reserved_balance += amount; + } else { + let total_reserve = <::Currency as ReservableCurrency< + T::AccountId, + >>::reserved_balance(&staker); + pre_state.insert(staker, (total_reserve, amount)); + } + }) + }); + + Ok(pre_state.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(pre_state: Vec) -> Result<(), &'static str> { + use sp_std::collections::btree_map::BTreeMap; + + if >::get() { + return Ok(()); } - ::WeightInfo::on_initialize(counter) + ensure!( + >::iter().collect::>().len() == 0, + "pendingUnstake storage isn't empty" + ); + + let mut is_ok = true; + + let pre_state: BTreeMap, BalanceOf)> = + Decode::decode(&mut &pre_state[..]).map_err(|_| "failed to decode pre_state")?; + for (staker, (total_reserved, reserved_by_promo)) in pre_state.into_iter() { + let new_state_reserve = <::Currency as ReservableCurrency< + T::AccountId, + >>::reserved_balance(&staker); + if new_state_reserve != total_reserved - reserved_by_promo { + is_ok = false; + log::error!( + "Incorrect reserved balance for {:?}. New balance: {:?}. Before runtime upgrade: total reserve - {:?}, reserved by promo - {:?}", + staker, new_state_reserve, total_reserved, reserved_by_promo + ); + } + } + + if is_ok { + Ok(()) + } else { + Err("Incorrect balance for some of stakers... See logs") + } } } @@ -340,14 +435,16 @@ <::Currency as Currency>::free_balance(&staker_id); // checks that we can lock `amount` on the `staker` account. - <::Currency as Currency>::ensure_can_withdraw( - &staker_id, - amount, - WithdrawReasons::all(), - balance - .checked_sub(&amount) - .ok_or(ArithmeticError::Underflow)?, - )?; + ensure!( + amount + <= match Self::get_locked_balance(&staker_id) { + Some(lock) => balance + .checked_sub(&lock.amount) + .ok_or(ArithmeticError::Underflow)?, + None => balance, + }, + ArithmeticError::Underflow + ); Self::add_lock_balance(&staker_id, amount)?; @@ -428,13 +525,6 @@ >::insert(block, pendings); - Self::unlock_balance(&staker_id, total_staked)?; - - <::Currency as ReservableCurrency>::reserve( - &staker_id, - total_staked, - )?; - TotalStaked::::set( TotalStaked::::get() .checked_sub(&total_staked) @@ -717,26 +807,6 @@ /// value and only call this once. pub fn account_id() -> T::AccountId { T::PalletId::get().into_account_truncating() - } - - /// Unlocks the balance that was locked by the pallet. - /// - /// - `staker`: staker account. - /// - `amount`: amount of unlocked funds. - fn unlock_balance(staker: &T::AccountId, amount: BalanceOf) -> DispatchResult { - let locked_balance = Self::get_locked_balance(staker) - .map(|l| l.amount) - .ok_or(>::IncorrectLockedBalanceOperation)?; - - // It is understood that we cannot unlock more funds than were locked by staking. - // Therefore, if implemented correctly, this error should not occur. - Self::set_lock_unchecked( - staker, - locked_balance - .checked_sub(&amount) - .ok_or(ArithmeticError::Underflow)?, - ); - Ok(()) } /// Adds the balance to locked by the pallet. --- a/tests/src/sub/appPromotion/appPromotion.test.ts +++ b/tests/src/sub/appPromotion/appPromotion.test.ts @@ -91,15 +91,48 @@ expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.equal(100n * nominal); }); - itSub('should reject transaction if stake amount is more than total free balance minus frozen', async ({helper}) => { + itSub('should allow to stake() if balance is locked with different id', async ({helper}) => { + const staker = accounts.pop()!; + + // staker has tokens locked with vesting id: + await helper.balance.vestedTransfer(donor, staker.address, {start: 0n, period: 1n, periodCount: 1n, perPeriod: 200n * nominal}); + expect(await helper.balance.getSubstrateFull(staker.address)) + .to.deep.contain({free: 1200n * nominal, miscFrozen: 200n * nominal, feeFrozen: 200n * nominal, reserved: 0n}); + + // Locked balance can be staked. staker can stake 1200 tokens (minus fee): + await helper.staking.stake(staker, 1000n * nominal); + await helper.staking.stake(staker, 199n * nominal); + // check balances + expect(await helper.balance.getLocked(staker.address)).to.deep.eq([{id: 'ormlvest', amount: 200n * nominal, reasons: 'All'}, {id: 'appstake', amount: 1199n * nominal, reasons: 'All'}]); + expect(await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 0n, miscFrozen: 1199n * nominal, feeFrozen: 1199n * nominal}); + expect(await helper.balance.getSubstrate(staker.address) / nominal).to.eq(1199n); + expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.eq(1199n * nominal); + + // staker can unstake + await helper.staking.unstake(staker); + expect(await helper.staking.getPendingUnstake({Substrate: staker.address})).to.eq(1199n * nominal); + const [pendingUnstake] = await helper.staking.getPendingUnstakePerBlock({Substrate: staker.address}); + await helper.wait.forParachainBlockNumber(pendingUnstake.block); + + // check balances + expect(await helper.balance.getLocked(staker.address)).to.deep.eq([{id: 'ormlvest', amount: 200n * nominal, reasons: 'All'}]); + expect(await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 0n, miscFrozen: 200n * nominal, feeFrozen: 200n * nominal}); + expect(await helper.balance.getSubstrate(staker.address) / nominal).to.eq(1199n); + expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.eq(0n); + + // staker can transfer balances now + await helper.balance.transferToSubstrate(staker, donor.address, 900n * nominal); + }); + + itSub('should not allow to stake(), if stake amount is more than total free balance minus locked by staking', async ({helper}) => { const staker = accounts.pop()!; // Can't stake full balance because Alice needs to pay some fee - await expect(helper.staking.stake(staker, 1000n * nominal)).to.be.rejected; + await expect(helper.staking.stake(staker, 1000n * nominal)).to.be.rejected; // With('Arithmetic') await helper.staking.stake(staker, 500n * nominal); // Can't stake 500 tkn because Alice has Less than 500 transferable; - await expect(helper.staking.stake(staker, 500n * nominal)).to.be.rejectedWith('balances.LiquidityRestrictions'); + await expect(helper.staking.stake(staker, 500n * nominal)).to.be.rejected; // With('Arithmetic'); expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.be.equal(500n * nominal); }); @@ -115,16 +148,17 @@ }); describe('unstake extrinsic', () => { - itSub('should change balance state from "frozen" to "reserved", add it to "pendingUnstake" map, and subtract it from totalStaked', async ({helper}) => { + itSub('should move tokens to "pendingUnstake" map and subtract it from totalStaked', async ({helper}) => { const [staker, recepient] = [accounts.pop()!, accounts.pop()!]; const totalStakedBefore = await helper.staking.getTotalStaked(); await helper.staking.stake(staker, 900n * nominal); await helper.staking.unstake(staker); - // Right after unstake balance is reserved + // Right after unstake tokens are still locked + expect(await helper.balance.getLocked(staker.address)).to.deep.eq([{id: 'appstake', amount: 900n * nominal, reasons: 'All'}]); + expect(await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 0n, miscFrozen: 900n * nominal, feeFrozen: 900n * nominal}); // Staker can not transfer - expect(await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 900n * nominal, miscFrozen: 0n, feeFrozen: 0n}); - await expect(helper.balance.transferToSubstrate(staker, recepient.address, 100n * nominal)).to.be.rejectedWith('balances.InsufficientBalance'); + await expect(helper.balance.transferToSubstrate(staker, recepient.address, 100n * nominal)).to.be.rejectedWith('balances.LiquidityRestrictions'); expect(await helper.staking.getPendingUnstake({Substrate: staker.address})).to.be.equal(900n * nominal); expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.be.equal(0n); expect(await helper.staking.getTotalStaked()).to.be.equal(totalStakedBefore); @@ -171,7 +205,8 @@ expect(stakes).to.be.deep.equal([]); expect(pendingUnstake[0].amount).to.equal(600n * nominal); - expect (await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 600n * nominal, feeFrozen: 0n, miscFrozen: 0n}); + expect (await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 0n, feeFrozen: 600n * nominal, miscFrozen: 600n * nominal}); + expect (await helper.balance.getSubstrate(staker.address) / nominal).to.be.equal(999n); await helper.wait.forParachainBlockNumber(pendingUnstake[0].block); expect (await helper.balance.getSubstrateFull(staker.address)).to.deep.contain({reserved: 0n, feeFrozen: 0n, miscFrozen: 0n}); expect (await helper.balance.getSubstrate(staker.address) / nominal).to.be.equal(999n); @@ -209,7 +244,7 @@ expect(unstakingPerBlock[1].amount).to.equal(120n * nominal); }); - itSub('should be possible for different accounts in one block', async ({helper}) => { + itSub('should be possible for 3 accounts in one block', async ({helper}) => { const stakers = [accounts.pop()!, accounts.pop()!, accounts.pop()!]; await Promise.all(stakers.map(staker => helper.staking.stake(staker, 100n * nominal))); @@ -220,6 +255,18 @@ expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.be.equal(0n); })); }); + + itSub('should not be possible for more than 3 accounts in one block', async ({helper}) => { + if (!await helper.arrange.isDevNode()) { + const stakers = await helper.arrange.createAccounts([200n,200n,200n,200n,200n,200n,200n,200n,200n,200n], donor); + + await Promise.all(stakers.map(staker => helper.staking.stake(staker, 100n * nominal))); + const unstakingResults = await Promise.allSettled(stakers.map(staker => helper.staking.unstake(staker))); + + const successfulUnstakes = unstakingResults.filter(result => result.status === 'fulfilled'); + expect(successfulUnstakes).to.have.length(3); + } + }); }); describe('collection sponsoring', () => { @@ -598,8 +645,8 @@ expect(stakerFullBalance).to.contain({reserved: 0n, feeFrozen: frozenBalanceShouldBe, miscFrozen: frozenBalanceShouldBe}); }); - itSub('should not be credited for unstaked (reserved) balance', async ({helper}) => { - // staker unstakes before rewards has been payed + itSub('should not be credited for pending-unstaked tokens', async ({helper}) => { + // staker unstakes before rewards been payed const staker = accounts.pop()!; await helper.staking.stake(staker, 100n * nominal); const [stake] = await helper.staking.getTotalStakedPerBlock({Substrate: staker.address});