From 4ba894504997c358811bf8cb0f50e119ae25503a Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 13 Aug 2024 19:24:36 +0530 Subject: [PATCH 1/6] fix: unstake active validators --- contracts/staking/stakeManager/StakeManager.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index 877ea40f..d5325adc 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -200,10 +200,12 @@ contract StakeManager is // Housekeeping function. @todo remove later function forceUnstake(uint256 validatorId) external onlyGovernance { + require(validators[validatorId].deactivationEpoch == 0); _unstake(validatorId, currentEpoch, false); } function forceUnstakePOL(uint256 validatorId) external onlyGovernance { + require(validators[validatorId].deactivationEpoch == 0); _unstake(validatorId, currentEpoch, true); } From 4b18cbf812eb7f2921d8f0518e6d3995638f42ec Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 13 Aug 2024 19:36:22 +0530 Subject: [PATCH 2/6] chore: rename to avoid collision with `_unstake(valId,exitEpoch,pol)` method --- contracts/staking/stakeManager/StakeManager.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index d5325adc..ce1a26a3 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -421,14 +421,14 @@ contract StakeManager is } function unstake(uint256 validatorId) external onlyStaker(validatorId) { - _unstake(validatorId, false); + _unstakeValidator(validatorId, false); } function unstakePOL(uint256 validatorId) external onlyStaker(validatorId) { - _unstake(validatorId, true); + _unstakeValidator(validatorId, true); } - function _unstake(uint256 validatorId, bool pol) internal { + function _unstakeValidator(uint256 validatorId, bool pol) internal { require(validatorAuction[validatorId].amount == 0); Status status = validators[validatorId].status; From 364a8c2837ee816ce4b63e620a0c69ebefcdd7c6 Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 13 Aug 2024 19:36:58 +0530 Subject: [PATCH 3/6] chore: run ci on feat/* pr --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7a992b4..470a94c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: push: branches: [main, dev] pull_request: - branches: [main, dev] + branches: [main, dev, feat/*] jobs: build: From bce13533d930217468f1f2dd2748208554a77a78 Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:37:30 +0530 Subject: [PATCH 4/6] test: non-idempotentcy of forceUnstake --- .../stakeManager/StakeManager.Staking.js | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/units/staking/stakeManager/StakeManager.Staking.js b/test/units/staking/stakeManager/StakeManager.Staking.js index 29187610..63a0b9bc 100644 --- a/test/units/staking/stakeManager/StakeManager.Staking.js +++ b/test/units/staking/stakeManager/StakeManager.Staking.js @@ -410,6 +410,58 @@ describe('unstake', function () { }) }) + describe('forceUnstake cannot only unstake active validators', function () { + const AliceWallet = wallets[1] + const others = [wallets[2], wallets[3]] + + before(async function() { + await freshDeploy.call(this, true) + }) + before(doStake(AliceWallet)) + before(doStake(others[0])) + before(doStake(others[1])) + before('Alice is forceUnstaked', async function () { + this.validatorId = await this.stakeManager.getValidatorId(AliceWallet.getAddressString()) + await this.governance.update( + this.stakeManager.address, + this.stakeManager.interface.encodeFunctionData('forceUnstake', [this.validatorId]) + ) + this.lastSyncedEpoch = await this.stakeManager.currentEpoch() + await checkPoint(others, this.rootChainOwner, this.stakeManager) + }) + it('Alice is unstaked', async function () { + const { deactivationEpoch } = await this.stakeManager.validators(this.validatorId) + assertBigNumberEquality(deactivationEpoch, this.lastSyncedEpoch) + }) + it('Alice cannot be forceUnstaked again', async function () { + await expectRevert(this.governance.update( + this.stakeManager.address, + this.stakeManager.interface.encodeFunctionData('forceUnstake', [this.validatorId]) + ), 'Update failed') + }) + it('Alice cannot be forceUnstaked after claiming', async function () { + const endEpoch = this.lastSyncedEpoch.add(await this.stakeManager.WITHDRAWAL_DELAY()) + // mock for i ... range(delay) checkPoint() + await this.governance.update( + this.stakeManager.address, + this.stakeManager.interface.encodeFunctionData('setCurrentEpoch', [endEpoch]) + ) + + await this.stakeManager + .connect(this.stakeManager.provider.getSigner(AliceWallet.getAddressString())) + .unstakeClaim(this.validatorId) + await checkPoint(others, this.rootChainOwner, this.stakeManager) + + await expectRevert( + this.governance.update( + this.stakeManager.address, + this.stakeManager.interface.encodeFunctionData('forceUnstake', [this.validatorId]) + ), + 'Update failed' + ) + }) + }) + describe('when user unstakes right after stake', async function () { const user = wallets[2].getChecksumAddressString() const amounts = walletAmounts[wallets[2].getAddressString()] From bd17618c0d0bcbcb5d14d1ad8d9f7eb53384aa0e Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:40:05 +0530 Subject: [PATCH 5/6] test: make robust --- test/units/staking/stakeManager/StakeManager.Staking.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/staking/stakeManager/StakeManager.Staking.js b/test/units/staking/stakeManager/StakeManager.Staking.js index 63a0b9bc..da71b106 100644 --- a/test/units/staking/stakeManager/StakeManager.Staking.js +++ b/test/units/staking/stakeManager/StakeManager.Staking.js @@ -444,7 +444,7 @@ describe('unstake', function () { // mock for i ... range(delay) checkPoint() await this.governance.update( this.stakeManager.address, - this.stakeManager.interface.encodeFunctionData('setCurrentEpoch', [endEpoch]) + this.stakeManager.interface.encodeFunctionData('setCurrentEpoch', [endEpoch + 1]) ) await this.stakeManager From 97cb2ef464762e7d80a947c3aab610f6278d6299 Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:43:26 +0530 Subject: [PATCH 6/6] fix: move check internally to cover for dethroneAndUnstake --- contracts/staking/stakeManager/StakeManager.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index ce1a26a3..5132ffd8 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -200,12 +200,10 @@ contract StakeManager is // Housekeeping function. @todo remove later function forceUnstake(uint256 validatorId) external onlyGovernance { - require(validators[validatorId].deactivationEpoch == 0); _unstake(validatorId, currentEpoch, false); } function forceUnstakePOL(uint256 validatorId) external onlyGovernance { - require(validators[validatorId].deactivationEpoch == 0); _unstake(validatorId, currentEpoch, true); } @@ -1114,6 +1112,7 @@ contract StakeManager is } function _unstake(uint256 validatorId, uint256 exitEpoch, bool pol) internal { + require(validators[validatorId].deactivationEpoch == 0); // TODO: if validators unstake and slashed to 0, he will be forced to unstake again // must think how to handle it correctly _updateRewards(validatorId);