From 2869dd3bac1283b68b572568904d0bd97b99d1ca Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Sep 2019 20:32:49 -0700 Subject: [PATCH] Removed unnecessary sloads in MixinStakingPoolMakers --- .../staking_pools/MixinCumulativeRewards.sol | 1 + .../src/staking_pools/MixinStakingPool.sol | 11 +--------- .../staking_pools/MixinStakingPoolMakers.sol | 20 ++++++------------- .../MixinStakingPoolModifiers.sol | 6 +++--- contracts/staking/package.json | 2 +- 5 files changed, 12 insertions(+), 28 deletions(-) diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index aed8acc133..53dfb4b88e 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -130,6 +130,7 @@ contract MixinCumulativeRewards is /// @dev Returns info on most recent cumulative reward. function _getMostRecentCumulativeRewardInfo(bytes32 poolId) internal + view returns (IStructs.CumulativeRewardInfo memory) { // fetch the last epoch at which we stored a cumulative reward for this pool diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index f4ff80b55a..4f3ce21f8e 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -103,16 +103,6 @@ contract MixinStakingPool is ); } - /// @dev Returns the unique id that will be assigned to the next pool that is created. - /// @return Pool id. - function getNextStakingPoolId() - public - view - returns (bytes32) - { - return nextPoolId; - } - /// @dev Returns a staking pool /// @param poolId Unique id of pool. function getStakingPool(bytes32 poolId) @@ -162,6 +152,7 @@ contract MixinStakingPool is uint32 newOperatorShare ) private + pure { // sanity checks if (newOperatorShare > PPM_DENOMINATOR) { diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol index 71e01399e7..bf16479d21 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol @@ -144,17 +144,6 @@ contract MixinStakingPoolMakers is return poolJoinedByMakerAddress[makerAddress].confirmed; } - /// @dev Returns the current number of makers in a given pool. - /// @param poolId Unique id of pool. - /// @return Size of pool. - function getNumberOfMakersInStakingPool(bytes32 poolId) - public - view - returns (uint256) - { - return poolById[poolId].numberOfMakers; - } - /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. /// Note also that the maker must have previously called joinStakingPoolAsMaker. /// @param poolId Unique id of pool. @@ -165,6 +154,9 @@ contract MixinStakingPoolMakers is ) internal { + // cache pool for use throughout this function + IStructs.Pool memory pool = poolById[poolId]; + // Is the maker already in a pool? if (isMakerAssignedToStakingPool(makerAddress)) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( @@ -176,7 +168,7 @@ contract MixinStakingPoolMakers is // Is the maker trying to join this pool; or are they the operator? bytes32 makerPendingPoolId = poolJoinedByMakerAddress[makerAddress].poolId; - if (makerPendingPoolId != poolId && makerAddress != poolById[poolId].operator) { + if (makerPendingPoolId != poolId && makerAddress != pool.operator) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, makerAddress, @@ -185,7 +177,7 @@ contract MixinStakingPoolMakers is } // Is the pool already full? - if (getNumberOfMakersInStakingPool(poolId) == maximumMakersInPool) { + if (pool.numberOfMakers == maximumMakersInPool) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( LibStakingRichErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, makerAddress, @@ -199,7 +191,7 @@ contract MixinStakingPoolMakers is confirmed: true }); poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - poolById[poolId].numberOfMakers = uint256(poolById[poolId].numberOfMakers).safeAdd(1).downcastToUint32(); + poolById[poolId].numberOfMakers = uint256(pool.numberOfMakers).safeAdd(1).downcastToUint32(); // Maker has been added to the pool emit MakerAddedToStakingPool( diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol index 71be9b68dc..7892f0c9af 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol @@ -44,10 +44,10 @@ contract MixinStakingPoolModifiers is /// @param poolId Pool sender must be operator of. /// @param makerAddress Address of a maker in the pool. modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId, address makerAddress) { - address operator; + address operator = poolById[poolId].operator; if ( - msg.sender != makerAddress && - msg.sender != (operator = poolById[poolId].operator) + msg.sender != operator && + msg.sender != makerAddress ) { LibRichErrors.rrevert( LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( diff --git a/contracts/staking/package.json b/contracts/staking/package.json index 7664ef0613..092fdb2502 100644 --- a/contracts/staking/package.json +++ b/contracts/staking/package.json @@ -37,7 +37,7 @@ }, "config": { "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.", - "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStorage|IStorageInit|IStructs|IVaultCore|IZrxVault|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestCumulativeRewardTracking|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestProtocolFees|TestProtocolFeesERC20Proxy|TestStaking|TestStakingProxy|TestStorageLayout|ZrxVault).json" + "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStorage|IStorageInit|IStructs|IVaultCore|IZrxVault|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolMakers|MixinStakingPoolModifiers|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestCumulativeRewardTracking|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestProtocolFees|TestProtocolFeesERC20Proxy|TestStaking|TestStakingProxy|TestStorageLayout|ZrxVault).json" }, "repository": { "type": "git",