From e062b2fd30799b978dbb7354bea97e16274fce14 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 16:39:45 +0700 Subject: [PATCH 1/7] fix: correctly calculate withdrawal shares --- contracts/staking/validatorShare/ValidatorShare.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index 9bd22372f..97229060e 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -248,6 +248,7 @@ contract ValidatorShare is IValidatorShare, ERC20NonTransferable, OwnableLockabl withdrawPool = withdrawPool.add(claimAmount); uint256 _withdrawPoolShare = claimAmount.mul(precision).div(withdrawExchangeRate()); + withdrawPool = withdrawPool.add(claimAmount); withdrawShares = withdrawShares.add(_withdrawPoolShare); DelegatorUnbond memory unbond = unbonds[msg.sender]; From 1d9d6c1f7869eab745c543982d654e31ce123302 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 16:41:45 +0700 Subject: [PATCH 2/7] fix: remove line --- contracts/staking/validatorShare/ValidatorShare.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index 97229060e..0357ddd18 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -245,7 +245,6 @@ contract ValidatorShare is IValidatorShare, ERC20NonTransferable, OwnableLockabl stakeManager.updateValidatorState(validatorId, -int256(claimAmount)); _reduceActiveStake(claimAmount); - withdrawPool = withdrawPool.add(claimAmount); uint256 _withdrawPoolShare = claimAmount.mul(precision).div(withdrawExchangeRate()); withdrawPool = withdrawPool.add(claimAmount); From a944aa282ac5a0cf0c1f535f1e625f8178c4f559 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 17:08:32 +0700 Subject: [PATCH 3/7] fix: broken withdrawal rate --- contracts/staking/validatorShare/ValidatorShare.sol | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index 0357ddd18..c8a5c338b 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -147,8 +147,15 @@ contract ValidatorShare is IValidatorShare, ERC20NonTransferable, OwnableLockabl } function withdrawExchangeRate() public view returns (uint256) { - uint256 _withdrawShares = withdrawShares; uint256 precision = _getRatePrecision(); + if (validatorId < 8) { + // fix of potentially broken withdrawals for future unbonding + // foundation validators have no slashing enabled and thus we can return default exchange rate + // because without slashing rate will stay constant + return precision; + } + + uint256 _withdrawShares = withdrawShares; return _withdrawShares == 0 ? precision From cbc5a01f859a7f49fbae3122c3c9959fc323fa18 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 18:53:37 +0700 Subject: [PATCH 4/7] upd: sell test --- test/units/staking/ValidatorShare.test.js | 145 ++++++++++++++++------ 1 file changed, 104 insertions(+), 41 deletions(-) diff --git a/test/units/staking/ValidatorShare.test.js b/test/units/staking/ValidatorShare.test.js index 71a8c69d6..715731230 100644 --- a/test/units/staking/ValidatorShare.test.js +++ b/test/units/staking/ValidatorShare.test.js @@ -9,10 +9,11 @@ const toWei = web3.utils.toWei const ZeroAddr = '0x0000000000000000000000000000000000000000' const ExchangeRatePrecision = new BN('100000000000000000000000000000') const Dynasty = 8 +const ValidatorDefaultStake = new BN(toWei('100')) -function shouldHaveCorrectStakes({ userTotalStaked, totalStaked }) { +function shouldHaveCorrectStakes({ user, userTotalStaked, totalStaked }) { it('must have correct total staked', async function() { - const result = await this.validatorContract.amountStaked(this.user) + const result = await this.validatorContract.amountStaked(user || this.user) assertBigNumberEquality(result, userTotalStaked) }) @@ -106,7 +107,7 @@ contract('ValidatorShare', async function() { this.validatorId = '8' this.validatorUser = wallets[0] - this.stakeAmount = new BN(toWei('100')) + this.stakeAmount = ValidatorDefaultStake await this.stakeManager.updateDynastyValue(Dynasty) await this.stakeManager.updateValidatorThreshold(8) @@ -392,7 +393,7 @@ contract('ValidatorShare', async function() { describe('2nd purchase', async function() { advanceCheckpointAfter() - + testBuyVoucher({ voucherValue: toWei('150'), voucherValueExpected: toWei('150'), @@ -739,35 +740,47 @@ contract('ValidatorShare', async function() { describe('sellVoucher', function() { const aliceStake = new BN(toWei('100')) + const bobStake = new BN(toWei('200')) const Alice = wallets[2].getChecksumAddressString() + const Bob = wallets[1].getChecksumAddressString() - async function doDeployAndBuyVoucherForAlice() { + async function doDeployAndBuyVoucherForAliceAndBob(includeBob = false) { await doDeploy.call(this) - this.user = wallets[2].getAddressString() - await this.stakeToken.mint( - this.user, - aliceStake - ) - - await this.stakeToken.approve(this.stakeManager.address, aliceStake, { - from: this.user - }) - - await buyVoucher(this.validatorContract, aliceStake, this.user) + const stake = async({ user, stake }) => { + await this.stakeToken.mint(user, stake) + await this.stakeToken.approve(this.stakeManager.address, stake, { + from: user + }) + await buyVoucher(this.validatorContract, stake, user) + } - this.userOldBalance = await this.stakeToken.balanceOf(this.user) - this.shares = await this.validatorContract.balanceOf(this.user) + await stake({ user: Alice, stake: aliceStake }) + if (includeBob) { + await stake({ user: Bob, stake: bobStake }) + } + for (let i = 0; i < 4; i++) { await checkPoint([this.validatorUser], this.rootChainOwner, this.stakeManager) } } - function testSellVoucher({ returnedStake, reward, initialBalance, validatorId, user, minClaimAmount, userTotalStaked, totalStaked, shares }) { + function testSellVoucher({ + returnedStake, + reward, + initialBalance, + validatorId, + user, + minClaimAmount, + userTotalStaked, + totalStaked, + shares, + withdrawalExchangeRate = ExchangeRatePrecision + }) { if (minClaimAmount) { it('must sell voucher with slippage', async function() { - this.receipt = await sellVoucher(this.validatorContract, this.user, minClaimAmount) + this.receipt = await sellVoucher(this.validatorContract, user, minClaimAmount) }) } else { it('must sell voucher', async function() { @@ -778,7 +791,7 @@ contract('ValidatorShare', async function() { it('must emit ShareBurned', async function() { await expectEvent.inTransaction(this.receipt.tx, StakingInfo, 'ShareBurned', { validatorId: validatorId, - tokens: shares || this.shares, + tokens: shares, amount: returnedStake, user: user }) @@ -788,12 +801,18 @@ contract('ValidatorShare', async function() { shouldHaveCorrectStakes({ userTotalStaked, - totalStaked + totalStaked, + user + }) + + it('must have correct withdrawal exchange rate', async function() { + const rate = await this.validatorContract.withdrawExchangeRate() + assertBigNumberEquality(rate, withdrawalExchangeRate) }) } describe('when Alice sells voucher', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) testSellVoucher({ returnedStake: aliceStake, @@ -802,7 +821,8 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: toWei('100') + totalStaked: ValidatorDefaultStake, + shares: aliceStake }) it('active amount must be == 0', async function() { @@ -811,24 +831,27 @@ contract('ValidatorShare', async function() { }) describe('when Alice sells voucher after 50% slash', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before('slash', async function() { await slash.call(this, [{ validator: this.validatorId, amount: this.stakeAmount }], [this.validatorUser], this.validatorUser) }) + const halfStake = aliceStake.div(new BN(2)) + testSellVoucher({ - returnedStake: aliceStake.div(new BN(2)), + returnedStake: halfStake, reward: toWei('18000'), initialBalance: new BN(0), validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: toWei('50') + totalStaked: ValidatorDefaultStake.div(new BN(2)), + shares: halfStake }) }) describe('when all tokens are slashed', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before('slash', async function() { await slash.call(this, [{ validator: this.validatorId, amount: await this.stakeManager.currentValidatorSetTotalStake() }], [this.validatorUser], this.validatorUser) }) @@ -839,7 +862,7 @@ contract('ValidatorShare', async function() { }) describe('when delegation is disabled after voucher was purchased by Alice', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before('disable delegation', async function() { await this.governance.update( this.stakeManager.address, @@ -854,12 +877,13 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: toWei('100') + totalStaked: ValidatorDefaultStake, + shares: aliceStake }) }) describe('when Alice sells with claimAmount greater than expected', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) it('reverts', async function() { const maxShares = await this.validatorContract.balanceOf(this.user) @@ -868,7 +892,7 @@ contract('ValidatorShare', async function() { }) describe('when locked', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before(async function() { await this.stakeManager.testLockShareContract(this.validatorId, true) @@ -881,12 +905,12 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: toWei('100') + totalStaked: ValidatorDefaultStake }) }) describe('when validator unstaked', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before(async function() { await this.stakeManager.unstake(this.validatorId, { from: this.validatorUser.getChecksumAddressString() }) await this.stakeManager.advanceEpoch(Dynasty) @@ -899,13 +923,51 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: toWei('100') + totalStaked: ValidatorDefaultStake + }) + }) + + describe('when Alice and Bob sell within withdrawal delay', function() { + before(async function() { + await doDeployAndBuyVoucherForAliceAndBob.call(this, true) + }) + + describe('when Alice sells', function() { + testSellVoucher({ + returnedStake: aliceStake, + reward: toWei('9000'), + initialBalance: new BN(0), + validatorId: '8', + user: Alice, + userTotalStaked: toWei('0'), + shares: aliceStake, + totalStaked: new BN(bobStake).add(ValidatorDefaultStake) + }) + }) + + describe('when Bob sells', function() { + testSellVoucher({ + returnedStake: bobStake, + reward: toWei('18000'), + initialBalance: new BN(0), + validatorId: '8', + user: Bob, + userTotalStaked: toWei('0'), + shares: bobStake, + totalStaked: ValidatorDefaultStake + }) + }) + + describe('after everyone sold', function() { + it('active amount must be == 0', async function() { + assertBigNumberEquality(await this.validatorContract.activeAmount(), '0') + }) }) }) describe('partial sell', function() { describe('when Alice is not slashed', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) const halfStake = aliceStake.div(new BN('2')) @@ -919,7 +981,7 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: halfStake, - totalStaked: halfStake.add(new BN(toWei('100'))) + totalStaked: halfStake.add(ValidatorDefaultStake) }) }) @@ -937,7 +999,7 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: '0', - totalStaked: toWei('100') + totalStaked: ValidatorDefaultStake }) it('unbond epoch must be set to current epoch', async function() { @@ -948,12 +1010,13 @@ contract('ValidatorShare', async function() { }) describe('when Alice is slashed by 50%', function() { - before(doDeployAndBuyVoucherForAlice) + before(doDeployAndBuyVoucherForAliceAndBob) before(async function() { await slash.call(this, [{ validator: this.validatorId, amount: toWei('100') }], [this.validatorUser], this.validatorUser, 1) }) const halfStake = aliceStake.div(new BN('4')) // slash by 50% occured + const validatorHalfStake = ValidatorDefaultStake.div(new BN(2)) describe('when Alice sells 50%', function() { testSellVoucher({ @@ -965,7 +1028,7 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: halfStake, - totalStaked: halfStake.add(new BN(toWei('50'))) + totalStaked: halfStake.add(validatorHalfStake) }) }) @@ -983,7 +1046,7 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: '0', - totalStaked: toWei('50') + totalStaked: validatorHalfStake }) it('unbond epoch must be set to current epoch', async function() { From 10c6592c7595c90dfca81ed6656ab4cb18caab6e Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 19:10:59 +0700 Subject: [PATCH 5/7] fix: tests --- test/units/staking/ValidatorShare.test.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/units/staking/ValidatorShare.test.js b/test/units/staking/ValidatorShare.test.js index 715731230..927b8e6a9 100644 --- a/test/units/staking/ValidatorShare.test.js +++ b/test/units/staking/ValidatorShare.test.js @@ -846,7 +846,7 @@ contract('ValidatorShare', async function() { user: Alice, userTotalStaked: toWei('0'), totalStaked: ValidatorDefaultStake.div(new BN(2)), - shares: halfStake + shares: aliceStake }) }) @@ -857,7 +857,7 @@ contract('ValidatorShare', async function() { }) it('reverts', async function() { - await expectRevert(sellVoucher(this.validatorContract, this.user, '0'), 'Too much requested') + await expectRevert(sellVoucher(this.validatorContract, Alice, '0'), 'Too much requested') }) }) @@ -887,7 +887,7 @@ contract('ValidatorShare', async function() { it('reverts', async function() { const maxShares = await this.validatorContract.balanceOf(this.user) - await expectRevert(this.validatorContract.sellVoucher(toWei('100.00001'), maxShares, { from: this.user }), 'Too much requested') + await expectRevert(this.validatorContract.sellVoucher(toWei('100.00001'), maxShares, { from: Alice }), 'Too much requested') }) }) @@ -905,7 +905,8 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: ValidatorDefaultStake + totalStaked: ValidatorDefaultStake, + shares: aliceStake }) }) @@ -923,7 +924,8 @@ contract('ValidatorShare', async function() { validatorId: '8', user: Alice, userTotalStaked: toWei('0'), - totalStaked: ValidatorDefaultStake + totalStaked: ValidatorDefaultStake, + shares: aliceStake }) }) From d8196abb9cca917e4f9fda13062e4c6b8ad5a1e2 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 27 Aug 2020 19:16:44 +0700 Subject: [PATCH 6/7] fix: test --- test/units/staking/ValidatorShare.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/staking/ValidatorShare.test.js b/test/units/staking/ValidatorShare.test.js index 927b8e6a9..0897e5a33 100644 --- a/test/units/staking/ValidatorShare.test.js +++ b/test/units/staking/ValidatorShare.test.js @@ -886,7 +886,7 @@ contract('ValidatorShare', async function() { before(doDeployAndBuyVoucherForAliceAndBob) it('reverts', async function() { - const maxShares = await this.validatorContract.balanceOf(this.user) + const maxShares = await this.validatorContract.balanceOf(Alice) await expectRevert(this.validatorContract.sellVoucher(toWei('100.00001'), maxShares, { from: Alice }), 'Too much requested') }) }) From 1f8996181ea9502139324809c5bf33e678802178 Mon Sep 17 00:00:00 2001 From: imyourm8 Date: Thu, 27 Aug 2020 20:07:29 +0700 Subject: [PATCH 7/7] misc: remove unrelated comment --- contracts/staking/validatorShare/ValidatorShare.sol | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index c8a5c338b..fffff4972 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -79,16 +79,6 @@ contract ValidatorShare is IValidatorShare, ERC20NonTransferable, OwnableLockabl onlyOwner returns (uint256) { - /** - restaking is simply buying more shares of pool - but those needs to be nonswapable/transferrable(to prevent https://en.wikipedia.org/wiki/Tragedy_of_the_commons) - - - calculate rewards for validator stake + delgation - - keep the validator rewards aside - - take the commission out - - add rewards to pool rewards - - returns total active stake for validator - */ uint256 combinedStakePower = validatorStake.add(activeAmount); // validator + delegation stake power uint256 rewards = combinedStakePower.mul(reward).div(checkpointStakePower);