git.delta.rocks / unique-network / refs/commits / f1a5ba762b0b

difftreelog

Merge pull request #867 from UniqueNetwork/feature/appPromoReservationBalance

Yaroslav Bolyukin2023-02-02parents: #d6786e1 #2380650.patch.diff
in: master
Feature/app promo reservation balance

5 files changed

modifiedCargo.lockdiffbeforeafterboth
--- 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",
modifiedpallets/app-promotion/CHANGELOG.mddiffbeforeafterboth
before · pallets/app-promotion/CHANGELOG.md
1# Change Log23All notable changes to this project will be documented in this file.45<!-- bureaucrate goes here -->67## [0.1.3] - 2022-12-2589### Fixed1011- Benchmarks for `payoutStakers` and `unstake` extrinsics.1213## [0.1.2] - 2022-12-201415### Fixed1617- The behaviour of the `payoutStakers` extrinsic18  in which only one stake is calculated for the last processed staker.1920## [0.1.1] - 2022-12-132122### Added2324- The ability to configure pallet `app-promotion` via the `configuration` pallet.
after · pallets/app-promotion/CHANGELOG.md
1# Change Log23All notable changes to this project will be documented in this file.45<!-- bureaucrate goes here -->67## [0.1.4] - 2023-01-3189### Changed1011- Balance reservation when calling the unstake method is removed.12  Now the locked balance (by AppPromo) does not change until the unlock interval expires.1314## [0.1.3] - 2022-12-251516### Fixed1718- Benchmarks for `payoutStakers` and `unstake` extrinsics.1920## [0.1.2] - 2022-12-202122### Fixed2324- The behaviour of the `payoutStakers` extrinsic25  in which only one stake is calculated for the last processed staker.2627## [0.1.1] - 2022-12-132829### Added3031- The ability to configure pallet `app-promotion` via the `configuration` pallet.
modifiedpallets/app-promotion/Cargo.tomldiffbeforeafterboth
--- 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
modifiedpallets/app-promotion/src/lib.rsdiffbeforeafterboth
--- 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<T: Config> =
 		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<T: Config> =
 		StorageValue<Value = (T::AccountId, T::BlockNumber), QueryKind = OptionQuery>;
 
+	#[pallet::storage]
+	pub(crate) type UpgradedToReserves<T: Config> =
+		StorageValue<Value = bool, QueryKind = ValueQuery>;
+
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		/// 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);
+					});
+				});
+			}
+
+			<T as Config>::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 <UpgradedToReserves<T>>::get() {
+				add_weight(1, 0, Weight::zero());
+				return consumed_weight;
+			} else {
+				add_weight(1, 1, Weight::zero());
+				<UpgradedToReserves<T>>::set(true);
+			}
+			<PendingUnstake<T>>::drain().for_each(|(_, v)| {
+				add_weight(1, 1, Weight::zero());
+				v.into_iter().for_each(|(staker, amount)| {
 					<<T as Config>::Currency as ReservableCurrency<T::AccountId>>::unreserve(
 						&staker, amount,
 					);
+					add_weight(1, 1, Weight::zero());
 				});
+			});
+
+			consumed_weight
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+			use sp_std::collections::btree_map::BTreeMap;
+			if <UpgradedToReserves<T>>::get() {
+				return Ok(Default::default());
+			}
+			// Staker -> (total amount of reserved balance, reserved by promotion);
+			let mut pre_state: BTreeMap<T::AccountId, (BalanceOf<T>, BalanceOf<T>)> =
+				BTreeMap::new();
+
+			<PendingUnstake<T>>::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 = <<T as Config>::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<u8>) -> Result<(), &'static str> {
+			use sp_std::collections::btree_map::BTreeMap;
+
+			if <UpgradedToReserves<T>>::get() {
+				return Ok(());
 			}
 
-			<T as Config>::WeightInfo::on_initialize(counter)
+			ensure!(
+				<PendingUnstake<T>>::iter().collect::<Vec<_>>().len() == 0,
+				"pendingUnstake storage isn't empty"
+			);
+
+			let mut is_ok = true;
+
+			let pre_state: BTreeMap<T::AccountId, (BalanceOf<T>, BalanceOf<T>)> =
+				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 = <<T as Config>::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 @@
 				<<T as Config>::Currency as Currency<T::AccountId>>::free_balance(&staker_id);
 
 			// checks that we can lock `amount` on the `staker` account.
-			<<T as Config>::Currency as Currency<T::AccountId>>::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 @@
 
 			<PendingUnstake<T>>::insert(block, pendings);
 
-			Self::unlock_balance(&staker_id, total_staked)?;
-
-			<<T as Config>::Currency as ReservableCurrency<T::AccountId>>::reserve(
-				&staker_id,
-				total_staked,
-			)?;
-
 			TotalStaked::<T>::set(
 				TotalStaked::<T>::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<T>) -> DispatchResult {
-		let locked_balance = Self::get_locked_balance(staker)
-			.map(|l| l.amount)
-			.ok_or(<Error<T>>::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.
modifiedtests/src/sub/appPromotion/appPromotion.test.tsdiffbeforeafterboth
--- 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});