From 2a3de5473dad742856a05bb4b2d7466a84fce3c4 Mon Sep 17 00:00:00 2001 From: PraetorP Date: Wed, 14 Dec 2022 10:20:32 +0000 Subject: [PATCH] fix: the behavior of the `appPromotion::payoutStakers` extrinsic, in which one staker could be skipped when called sequentially. --- --- a/pallets/app-promotion/src/lib.rs +++ b/pallets/app-promotion/src/lib.rs @@ -255,11 +255,11 @@ ValueQuery, >; - /// Stores a key for record for which the next revenue recalculation would be performed. + /// Stores a key for record for which the revenue recalculation was performed. /// If `None`, then recalculation has not yet been performed or calculations have been completed for all stakers. #[pallet::storage] #[pallet::getter(fn get_next_calculated_record)] - pub type NextCalculatedRecord = + pub type PreviousCalculatedRecord = StorageValue; #[pallet::hooks] @@ -577,7 +577,7 @@ let mut stakers_number = stakers_number.unwrap_or(DEFAULT_NUMBER_PAYOUTS); ensure!( - stakers_number <= config.max_stakers_per_calculation, + stakers_number <= config.max_stakers_per_calculation && stakers_number != 0, Error::::NoPermission ); @@ -595,7 +595,7 @@ let mut storage_iterator = Self::get_next_calculated_key() .map_or(Staked::::iter(), |key| Staked::::iter_from(key)); - NextCalculatedRecord::::set(None); + PreviousCalculatedRecord::::set(None); { let last_id = RefCell::new(None); @@ -640,10 +640,6 @@ (amount, next_recalc_block_for_stake), )) = storage_iterator.next() { - if stakers_number == 0 { - NextCalculatedRecord::::set(Some((current_id, staked_block))); - break; - } if last_id.borrow().as_ref() != Some(¤t_id) { flush_stake()?; *last_id.borrow_mut() = Some(current_id.clone()); @@ -662,6 +658,13 @@ &mut *income_acc.borrow_mut(), ); } + + if stakers_number == 0 { + if storage_iterator.next().is_some() { + PreviousCalculatedRecord::::set(Some((current_id, staked_block))); + } + break; + } } flush_stake()?; } --- a/tests/src/app-promotion.test.ts +++ b/tests/src/app-promotion.test.ts @@ -33,40 +33,21 @@ requirePalletsOrSkip(this, helper, [Pallets.AppPromotion]); donor = await privateKey({filename: __filename}); palletAddress = helper.arrange.calculatePalletAddress('appstake'); - palletAdmin = await privateKey('//PromotionAdmin'); + palletAdmin = await privateKey('//Alice'); nominal = helper.balance.getOneTokenNominal(); - const accountBalances = new Array(100); - accountBalances.fill(1000n); - accounts = await helper.arrange.createAccounts(accountBalances, donor); // create accounts-pool to speed up tests + await helper.executeExtrinsic(palletAdmin, 'api.tx.sudo.sudo', [helper.api!.tx.appPromotion.setAdminAddress({Substrate: palletAdmin.address})]); + await helper.executeExtrinsic(palletAdmin, 'api.tx.sudo.sudo', [helper.api!.tx.configuration + .setAppPromotionConfigurationOverride({ + recalculationInterval: LOCKING_PERIOD, + pendingInterval: UNLOCKING_PERIOD})], true); }); }); describe('stake extrinsic', () => { itSub('should "lock" staking balance, add it to "staked" map, and increase "totalStaked" amount', async ({helper}) => { - const [staker, recepient] = [accounts.pop()!, accounts.pop()!]; - const totalStakedBefore = await helper.staking.getTotalStaked(); - - // Minimum stake amount is 100: - await expect(helper.staking.stake(staker, 100n * nominal - 1n)).to.be.rejected; - await helper.staking.stake(staker, 100n * nominal); - - // Staker balance is: miscFrozen: 100, feeFrozen: 100, reserved: 0n... - // ...so he can not transfer 900 - expect (await helper.balance.getSubstrateFull(staker.address)).to.contain({miscFrozen: 100n * nominal, feeFrozen: 100n * nominal, reserved: 0n}); - await expect(helper.balance.transferToSubstrate(staker, recepient.address, 900n * nominal)).to.be.rejectedWith('balances.LiquidityRestrictions'); - - expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.be.equal(100n * nominal); - expect(await helper.balance.getSubstrate(staker.address) / nominal).to.be.equal(999n); - // it is potentially flaky test. Promotion can credited some tokens. Maybe we need to use closeTo? - expect(await helper.staking.getTotalStaked()).to.be.equal(totalStakedBefore + 100n * nominal); // total tokens amount staked in app-promotion increased - - - await helper.staking.stake(staker, 200n * nominal); - expect(await helper.staking.getTotalStaked({Substrate: staker.address})).to.be.equal(300n * nominal); - const totalStakedPerBlock = await helper.staking.getTotalStakedPerBlock({Substrate: staker.address}); - expect(totalStakedPerBlock[0].amount).to.equal(100n * nominal); - expect(totalStakedPerBlock[1].amount).to.equal(200n * nominal); + + }); itSub('should allow to create maximum 10 stakes for account', async ({helper}) => { --- a/tests/src/interfaces/augment-api-query.ts +++ b/tests/src/interfaces/augment-api-query.ts @@ -23,11 +23,6 @@ **/ admin: AugmentedQuery Observable>, []> & QueryableStorageEntry; /** - * Stores a key for record for which the next revenue recalculation would be performed. - * If `None`, then recalculation has not yet been performed or calculations have been completed for all stakers. - **/ - nextCalculatedRecord: AugmentedQuery Observable>>, []> & QueryableStorageEntry; - /** * Stores amount of stakes for an `Account`. * * * **Key** - Staker account. @@ -35,6 +30,11 @@ **/ pendingUnstake: AugmentedQuery Observable>>, [u32]> & QueryableStorageEntry; /** + * Stores a key for record for which the next revenue recalculation would be performed. + * If `None`, then recalculation has not yet been performed or calculations have been completed for all stakers. + **/ + previousCalculatedRecord: AugmentedQuery Observable>>, []> & QueryableStorageEntry; + /** * Stores the amount of tokens staked by account in the blocknumber. * * * **Key1** - Staker account. -- gitstuff