Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Fix 100% operator share #2333

Merged
merged 4 commits into from
Nov 13, 2019
Merged

Fix 100% operator share #2333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f3d281
Allow operator share to be set to existing share
abandeali1 Nov 13, 2019
d90810d
Add tests for setting operator share to 100%
abandeali1 Nov 13, 2019
41a34c1
Add zrxVault config tests in migrations
abandeali1 Nov 13, 2019
f619646
Add test for when operatorShare == 0
abandeali1 Nov 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ contract MixinStakingPool is
poolId,
newOperatorShare
));
} else if (newOperatorShare >= currentOperatorShare) {
// new share must be less than the current share
} else if (newOperatorShare > currentOperatorShare) {
// new share must be less than or equal to the current share
LibRichErrors.rrevert(LibStakingRichErrors.OperatorShareError(
LibStakingRichErrors.OperatorShareErrorCodes.CanOnlyDecreaseOperatorShare,
poolId,
Expand Down
9 changes: 2 additions & 7 deletions contracts/staking/test/pools_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ blockchainTests('Staking Pool Management', env => {
// decrease operator share
await poolOperator.decreaseStakingPoolOperatorShareAsync(poolId, increasedOperatorShare, revertError);
});
it('Should fail if operator calls decreaseStakingPoolOperatorShare but newOperatorShare == oldOperatorShare', async () => {
it('Should be successful if operator calls decreaseStakingPoolOperatorShare and newOperatorShare == oldOperatorShare', async () => {
// test parameters
const operatorAddress = users[0];
const operatorShare = (39 / 100) * PPM_DENOMINATOR;
Expand All @@ -184,13 +184,8 @@ blockchainTests('Staking Pool Management', env => {
const poolId = await poolOperator.createStakingPoolAsync(operatorShare, false);
expect(poolId).to.be.equal(stakingConstants.INITIAL_POOL_ID);

const revertError = new StakingRevertErrors.OperatorShareError(
StakingRevertErrors.OperatorShareErrorCodes.CanOnlyDecreaseOperatorShare,
poolId,
operatorShare,
);
// decrease operator share
await poolOperator.decreaseStakingPoolOperatorShareAsync(poolId, operatorShare, revertError);
await poolOperator.decreaseStakingPoolOperatorShareAsync(poolId, operatorShare);
});
it('should fail to decrease operator share if not called by operator', async () => {
// test parameters
Expand Down
36 changes: 22 additions & 14 deletions contracts/staking/test/unit_tests/staking_pool_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,20 @@ blockchainTests.resets('MixinStakingPool unit tests', env => {
expect(pool.operator).to.eq(operator);
expect(pool.operatorShare).to.bignumber.eq(operatorShare);
});
it('records pool details when operator share is 100%', async () => {
const operatorShare = constants.PPM_100_PERCENT;
await testContract.createStakingPool.awaitTransactionSuccessAsync(operatorShare, false, { from: operator });
const pool = await testContract.getStakingPool.callAsync(nextPoolId);
expect(pool.operator).to.eq(operator);
expect(pool.operatorShare).to.bignumber.eq(operatorShare);
});
it('records pool details when operator share is 0%', async () => {
const operatorShare = constants.ZERO_AMOUNT;
await testContract.createStakingPool.awaitTransactionSuccessAsync(operatorShare, false, { from: operator });
const pool = await testContract.getStakingPool.callAsync(nextPoolId);
expect(pool.operator).to.eq(operator);
expect(pool.operatorShare).to.bignumber.eq(operatorShare);
});
it('returns the next pool ID', async () => {
const poolId = await testContract.createStakingPool.callAsync(randomOperatorShare(), false, {
from: operator,
Expand Down Expand Up @@ -239,20 +253,6 @@ blockchainTests.resets('MixinStakingPool unit tests', env => {
const expectedError = new StakingRevertErrors.OnlyCallableByPoolOperatorError(maker, poolId);
return expect(tx).to.revertWith(expectedError);
});
it('fails if operator share is equal to current', async () => {
const { poolId, operatorShare } = await createPoolAsync();
const tx = testContract.decreaseStakingPoolOperatorShare.awaitTransactionSuccessAsync(
poolId,
operatorShare,
{ from: operator },
);
const expectedError = new StakingRevertErrors.OperatorShareError(
StakingRevertErrors.OperatorShareErrorCodes.CanOnlyDecreaseOperatorShare,
poolId,
operatorShare,
);
return expect(tx).to.revertWith(expectedError);
});
it('fails if operator share is greater than current', async () => {
const { poolId, operatorShare } = await createPoolAsync();
const tx = testContract.decreaseStakingPoolOperatorShare.awaitTransactionSuccessAsync(
Expand Down Expand Up @@ -291,6 +291,14 @@ blockchainTests.resets('MixinStakingPool unit tests', env => {
const pool = await testContract.getStakingPool.callAsync(poolId);
expect(pool.operatorShare).to.bignumber.eq(operatorShare - 1);
});
it('does not modify operator share if equal to current', async () => {
const { poolId, operatorShare } = await createPoolAsync();
await testContract.decreaseStakingPoolOperatorShare.awaitTransactionSuccessAsync(poolId, operatorShare, {
from: operator,
});
const pool = await testContract.getStakingPool.callAsync(poolId);
expect(pool.operatorShare).to.bignumber.eq(operatorShare);
});
it('does not modify operator', async () => {
const { poolId, operatorShare } = await createPoolAsync();
await testContract.decreaseStakingPoolOperatorShare.awaitTransactionSuccessAsync(
Expand Down
13 changes: 10 additions & 3 deletions packages/migrations/src/test_contract_configs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,6 @@ async function testContractConfigsAsync(provider: SupportedProvider): Promise<vo
const stakingProxyOwner = await stakingProxy.owner.callAsync();
warnIfMismatch(stakingProxyOwner, addresses.zeroExGovernor, 'Unexpected StakingProxy owner');

const zrxVaultOwner = await zrxVault.owner.callAsync();
warnIfMismatch(zrxVaultOwner, addresses.zeroExGovernor, 'Unexpected ZrxVault owner');

const stakingProxyAuthorizedAddresses = await stakingProxy.getAuthorizedAddresses.callAsync();
warnIfMismatch(
stakingProxyAuthorizedAddresses.length,
Expand All @@ -277,11 +274,21 @@ async function testContractConfigsAsync(provider: SupportedProvider): Promise<vo
const isGovernorAuthorizedInStakingProxy = await stakingProxy.authorized.callAsync(addresses.zeroExGovernor);
warnIfMismatch(isGovernorAuthorizedInStakingProxy, true, 'ZeroExGovernor not authorized in StakingProxy');

const zrxVaultOwner = await zrxVault.owner.callAsync();
warnIfMismatch(zrxVaultOwner, addresses.zeroExGovernor, 'Unexpected ZrxVault owner');

const zrxVaultAuthorizedAddresses = await zrxVault.getAuthorizedAddresses.callAsync();
warnIfMismatch(zrxVaultAuthorizedAddresses.length, 1, 'Unexpected number of authorized addresses in ZrxVault');

const isGovernorAuthorizedInZrxVault = await zrxVault.authorized.callAsync(addresses.zeroExGovernor);
warnIfMismatch(isGovernorAuthorizedInZrxVault, true, 'ZeroExGovernor not authorized in ZrxVault');

const zrxAssetProxy = await zrxVault.zrxAssetProxy.callAsync();
warnIfMismatch(zrxAssetProxy, addresses.erc20Proxy, 'Unexpected ERC20Proxy set in ZrxVault');

const zrxVaultStakingProxy = await zrxVault.stakingProxyAddress.callAsync();
warnIfMismatch(zrxVaultStakingProxy, addresses.stakingProxy, 'Unexpected StakingProxy set in ZrxVault');

const params = await stakingContract.getParams.callAsync();
warnIfMismatch(
params[0].toNumber(),
Expand Down