From 6410366f8b390ee1a6b27d06ba790e1ca4f20e5e Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Sep 2019 20:51:55 -0400 Subject: [PATCH] `@0x/contracts-staking`: Fix contracts formatting. `@0x/contracts-staking`: Remove typo test suite in `migration.ts`. `@0x/contracts-staking`: Address minor review comments. --- .../staking/contracts/src/ReadOnlyProxy.sol | 1 + contracts/staking/contracts/src/Staking.sol | 1 + .../staking/contracts/src/StakingProxy.sol | 1 + .../contracts/src/fees/MixinExchangeFees.sol | 1 + .../src/fees/MixinExchangeManager.sol | 1 + .../contracts/src/interfaces/IStaking.sol | 1 + .../contracts/src/interfaces/IStorageInit.sol | 1 + .../contracts/src/libs/LibFixedMath.sol | 1 + .../src/libs/LibStakingRichErrors.sol | 2 +- .../contracts/src/stake/MixinStake.sol | 1 + .../src/stake/MixinStakeBalances.sol | 1 + .../contracts/src/stake/MixinStakeStorage.sol | 1 + .../contracts/src/stake/MixinZrxVault.sol | 1 + .../src/staking_pools/MixinStakingPool.sol | 1 + .../staking_pools/MixinStakingPoolRewards.sol | 1 + .../staking/contracts/src/sys/MixinParams.sol | 3 +- .../contracts/src/sys/MixinScheduler.sol | 1 + .../staking/contracts/src/vaults/EthVault.sol | 1 + .../src/vaults/StakingPoolRewardVault.sol | 1 + .../staking/contracts/src/vaults/ZrxVault.sol | 1 + .../contracts/test/TestCobbDouglas.sol | 1 + .../contracts/test/TestExchangeFees.sol | 1 + .../staking/contracts/test/TestInitTarget.sol | 1 + .../contracts/test/TestStakingProxy.sol | 1 + .../contracts/test/TestStorageLayout.sol | 1 + contracts/staking/test/migration.ts | 30 +------------------ 26 files changed, 27 insertions(+), 31 deletions(-) diff --git a/contracts/staking/contracts/src/ReadOnlyProxy.sol b/contracts/staking/contracts/src/ReadOnlyProxy.sol index 4e3277c908..7613d07ec3 100644 --- a/contracts/staking/contracts/src/ReadOnlyProxy.sol +++ b/contracts/staking/contracts/src/ReadOnlyProxy.sol @@ -25,6 +25,7 @@ import "./libs/LibProxy.sol"; contract ReadOnlyProxy is MixinStorage { + using LibProxy for address; // solhint-disable payable-fallback diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index aef1ddeb65..cdfcc5b171 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -50,6 +50,7 @@ contract Staking is MixinStakingPool, MixinExchangeFees { + // this contract can receive ETH // solhint-disable no-empty-blocks function () diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 5e8dda892b..4f5d1743d5 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -33,6 +33,7 @@ contract StakingProxy is Ownable, MixinStorage { + using LibProxy for address; /// @dev Constructor. diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 4ecdb97420..dcfcd0a2a6 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -58,6 +58,7 @@ contract MixinExchangeFees is MixinStakingPoolRewards, MixinStakingPool { + using LibSafeMath for uint256; /// @dev Pays a protocol fee in ETH or WETH. diff --git a/contracts/staking/contracts/src/fees/MixinExchangeManager.sol b/contracts/staking/contracts/src/fees/MixinExchangeManager.sol index e481283c23..6249717d70 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeManager.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeManager.sol @@ -35,6 +35,7 @@ contract MixinExchangeManager is Ownable, MixinStorage { + /// @dev Asserts that the call is coming from a valid exchange. modifier onlyExchange() { if (!isValidExchangeAddress(msg.sender)) { diff --git a/contracts/staking/contracts/src/interfaces/IStaking.sol b/contracts/staking/contracts/src/interfaces/IStaking.sol index de784e0e4f..1f29133f3c 100644 --- a/contracts/staking/contracts/src/interfaces/IStaking.sol +++ b/contracts/staking/contracts/src/interfaces/IStaking.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.9; interface IStaking { + /// @dev Pays a protocol fee in ETH. /// @param makerAddress The address of the order's maker. /// @param payerAddress The address that is responsible for paying the protocol fee. diff --git a/contracts/staking/contracts/src/interfaces/IStorageInit.sol b/contracts/staking/contracts/src/interfaces/IStorageInit.sol index b8677d681b..e06fce9897 100644 --- a/contracts/staking/contracts/src/interfaces/IStorageInit.sol +++ b/contracts/staking/contracts/src/interfaces/IStorageInit.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.9; interface IStorageInit { + /// @dev Initialize storage owned by this contract. function init() external; } diff --git a/contracts/staking/contracts/src/libs/LibFixedMath.sol b/contracts/staking/contracts/src/libs/LibFixedMath.sol index cfe58695a5..956a8b80cd 100644 --- a/contracts/staking/contracts/src/libs/LibFixedMath.sol +++ b/contracts/staking/contracts/src/libs/LibFixedMath.sol @@ -24,6 +24,7 @@ import "./LibFixedMathRichErrors.sol"; // solhint-disable indent /// @dev Signed, fixed-point, 127-bit precision math library. library LibFixedMath { + // 1 int256 private constant FIXED_1 = int256(0x0000000000000000000000000000000080000000000000000000000000000000); // 1^2 (in fixed-point) diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 5e7f815c5d..f546d362e3 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -110,7 +110,7 @@ library LibStakingRichErrors { bytes4 internal constant POOL_ALREADY_EXISTS_ERROR_SELECTOR = 0x2a5e4dcf; - // bytes4(keccak256("EthVaultNotSetError()")) + // bytes4(keccak256("EthVaultNotSetError()")) bytes4 internal constant ETH_VAULT_NOT_SET_ERROR_SELECTOR = 0xa067f596; diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index c68c648623..2ce61bb30e 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -46,6 +46,7 @@ contract MixinStake is MixinStakeBalances, MixinStakingPoolRewards { + using LibSafeMath for uint256; /// @dev Stake ZRX tokens. Tokens are deposited into the ZRX Vault. Unstake to retrieve the ZRX. diff --git a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol index 95ce8a3fdd..6e88d8968d 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol @@ -39,6 +39,7 @@ contract MixinStakeBalances is MixinScheduler, MixinStakeStorage { + using LibSafeMath for uint256; /// @dev Returns the total stake for a given owner. diff --git a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol index b39ffa14aa..c52967a948 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol @@ -35,6 +35,7 @@ contract MixinStakeStorage is MixinStorage, MixinScheduler { + using LibSafeMath for uint256; using LibSafeDowncast for uint256; diff --git a/contracts/staking/contracts/src/stake/MixinZrxVault.sol b/contracts/staking/contracts/src/stake/MixinZrxVault.sol index 1a8e701831..2c11efe183 100644 --- a/contracts/staking/contracts/src/stake/MixinZrxVault.sol +++ b/contracts/staking/contracts/src/stake/MixinZrxVault.sol @@ -29,6 +29,7 @@ contract MixinZrxVault is Ownable, MixinStorage { + /// @dev Set the Zrx Vault. /// @param zrxVaultAddress Address of the Zrx Vault. function setZrxVault(address zrxVaultAddress) diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index aa191c2968..a749870817 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -64,6 +64,7 @@ contract MixinStakingPool is MixinStakeBalances, MixinStakingPoolRewards { + using LibSafeMath for uint256; /// @dev Create a new staking pool. The sender will be the operator of this pool. diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index d969bfd361..12ded65743 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -38,6 +38,7 @@ contract MixinStakingPoolRewards is MixinStakeStorage, MixinStakeBalances { + using LibSafeMath for uint256; /// @dev Computes the reward balance in ETH of a specific member of a pool. diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 9cbbba2f49..88adff12d8 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -32,6 +32,7 @@ contract MixinParams is Ownable, MixinStorage { + /// @dev Set all configurable parameters at once. /// @param _epochDurationInSeconds Minimum seconds between epochs. /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. @@ -74,7 +75,7 @@ contract MixinParams is ); } - /// @dev Retrives all configurable parameter values. + /// @dev Retrieves all configurable parameter values. /// @return _epochDurationInSeconds Minimum seconds between epochs. /// @return _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. /// @return _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. diff --git a/contracts/staking/contracts/src/sys/MixinScheduler.sol b/contracts/staking/contracts/src/sys/MixinScheduler.sol index a8a9e39623..baebd5b656 100644 --- a/contracts/staking/contracts/src/sys/MixinScheduler.sol +++ b/contracts/staking/contracts/src/sys/MixinScheduler.sol @@ -39,6 +39,7 @@ contract MixinScheduler is Ownable, MixinStorage { + using LibSafeMath for uint256; /// @dev Returns the current epoch. diff --git a/contracts/staking/contracts/src/vaults/EthVault.sol b/contracts/staking/contracts/src/vaults/EthVault.sol index 73cb712202..9625ed3ba9 100644 --- a/contracts/staking/contracts/src/vaults/EthVault.sol +++ b/contracts/staking/contracts/src/vaults/EthVault.sol @@ -30,6 +30,7 @@ contract EthVault is IVaultCore, MixinVaultCore { + using LibSafeMath for uint256; // mapping from Owner to ETH balance diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 0956b3a54e..ae0b199f08 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -46,6 +46,7 @@ contract StakingPoolRewardVault is MixinConstants, MixinVaultCore { + using LibSafeMath for uint256; using LibSafeDowncast for uint256; diff --git a/contracts/staking/contracts/src/vaults/ZrxVault.sol b/contracts/staking/contracts/src/vaults/ZrxVault.sol index 8733647207..05e9b63329 100644 --- a/contracts/staking/contracts/src/vaults/ZrxVault.sol +++ b/contracts/staking/contracts/src/vaults/ZrxVault.sol @@ -39,6 +39,7 @@ contract ZrxVault is IZrxVault, MixinVaultCore { + using LibSafeMath for uint256; // mapping from Owner to ZRX balance diff --git a/contracts/staking/contracts/test/TestCobbDouglas.sol b/contracts/staking/contracts/test/TestCobbDouglas.sol index 70c029d251..49b16b89ba 100644 --- a/contracts/staking/contracts/test/TestCobbDouglas.sol +++ b/contracts/staking/contracts/test/TestCobbDouglas.sol @@ -24,6 +24,7 @@ import "../src/fees/MixinExchangeFees.sol"; contract TestCobbDouglas is MixinExchangeFees { + function cobbDouglas( uint256 totalRewards, uint256 ownerFees, diff --git a/contracts/staking/contracts/test/TestExchangeFees.sol b/contracts/staking/contracts/test/TestExchangeFees.sol index b3cace57ea..98ac0af184 100644 --- a/contracts/staking/contracts/test/TestExchangeFees.sol +++ b/contracts/staking/contracts/test/TestExchangeFees.sol @@ -24,6 +24,7 @@ import "../src/Staking.sol"; contract TestExchangeFees is Staking { + struct TestPool { uint256 stake; mapping(address => bool) isMaker; diff --git a/contracts/staking/contracts/test/TestInitTarget.sol b/contracts/staking/contracts/test/TestInitTarget.sol index 3ce473446e..576869ceb0 100644 --- a/contracts/staking/contracts/test/TestInitTarget.sol +++ b/contracts/staking/contracts/test/TestInitTarget.sol @@ -26,6 +26,7 @@ contract TestInitTarget is Ownable, MixinStorage { + // We can't store state in this contract before it is attached, so // we will grant this predefined address a balance to indicate that // `init()` should revert. diff --git a/contracts/staking/contracts/test/TestStakingProxy.sol b/contracts/staking/contracts/test/TestStakingProxy.sol index add017e4ab..f071cf32c3 100644 --- a/contracts/staking/contracts/test/TestStakingProxy.sol +++ b/contracts/staking/contracts/test/TestStakingProxy.sol @@ -24,6 +24,7 @@ import "../src/StakingProxy.sol"; contract TestStakingProxy is StakingProxy { + // solhint-disable no-empty-blocks constructor(address _stakingContract) public diff --git a/contracts/staking/contracts/test/TestStorageLayout.sol b/contracts/staking/contracts/test/TestStorageLayout.sol index 694c21a88f..ba89e1f0d0 100644 --- a/contracts/staking/contracts/test/TestStorageLayout.sol +++ b/contracts/staking/contracts/test/TestStorageLayout.sol @@ -29,6 +29,7 @@ contract TestStorageLayout is Ownable, MixinStorage { + function assertExpectedStorageLayout() public pure diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index e1467e5a6e..b7ac88a45b 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -49,7 +49,7 @@ blockchainTests('Migration tests', env => { ...env.txDefaults, from: ownerAddress, to: revertAddress, - data: '0x', + data: constants.NULL_BYTES, value: new BigNumber(1), }), ); @@ -114,34 +114,6 @@ blockchainTests('Migration tests', env => { expect(initCounter).to.bignumber.eq(2); }); }); - - blockchainTests.resets('upgrade', async () => { - let proxyContract: TestStakingProxyContract; - - before(async () => { - proxyContract = await deployStakingProxyAsync(); - }); - - it('incm', async () => { - const attachedAddress = randomAddress(); - const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync(attachedAddress, { - from: notOwnerAddress, - }); - const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwnerAddress, ownerAddress); - return expect(tx).to.revertWith(expectedError); - }); - - it('calls init() and attaches the contract', async () => { - await proxyContract.attachStakingContract.awaitTransactionSuccessAsync(initTargetContract.address); - await assertInitStateAsync(proxyContract); - }); - - it('reverts if init() reverts', async () => { - await enableInitRevertsAsync(); - const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync(initTargetContract.address); - return expect(tx).to.revertWith(REVERT_ERROR); - }); - }); }); blockchainTests.resets('Staking.init()', async () => {