diff --git a/contracts/staking/contracts/src/ReadOnlyProxy.sol b/contracts/staking/contracts/src/ReadOnlyProxy.sol index 08cc52b444..32a3ae5b43 100644 --- a/contracts/staking/contracts/src/ReadOnlyProxy.sol +++ b/contracts/staking/contracts/src/ReadOnlyProxy.sol @@ -18,52 +18,28 @@ pragma solidity ^0.5.9; +import "./immutable/MixinConstants.sol"; import "./immutable/MixinStorage.sol"; +import "./libs/LibProxy.sol"; contract ReadOnlyProxy is MixinStorage { + using LibProxy for address; + /// @dev Executes a read-only call to the staking contract, via `revertDelegateCall`. /// By routing through `revertDelegateCall` any state changes are reverted. // solhint-disable no-complex-fallback function () external { - address thisAddress = address(this); - bytes4 revertDelegateCallSelector = this.revertDelegateCall.selector; - - assembly { - // store selector of destination function - mstore(0x0, revertDelegateCallSelector) - - // copy calldata to memory - calldatacopy( - 0x4, - 0x0, - calldatasize() - ) - - // delegate call into staking contract - let success := delegatecall( - gas, // forward all gas - thisAddress, // calling staking contract - 0x0, // start of input (calldata) - add(calldatasize(), 4), // length of input (calldata) - 0x0, // write output over input - 0 // length of output is unknown - ) - - // copy return data to memory and *return* - returndatacopy( - 0x0, - 0x0, - returndatasize() - ) - - return(0, returndatasize()) - } + address(this).proxyCall( + LibProxy.RevertRule.NEVER_REVERT, + this.revertDelegateCall.selector, + false // do not ignore this selector + ); } /// @dev Executes a delegate call to the staking contract, if it is set. @@ -71,37 +47,10 @@ contract ReadOnlyProxy is function revertDelegateCall() external { - address _readOnlyProxyCallee = readOnlyProxyCallee; - if (_readOnlyProxyCallee == address(0)) { - return; - } - - assembly { - // copy calldata to memory - calldatacopy( - 0x0, - 0x4, - calldatasize() - ) - - // delegate call into staking contract - let success := delegatecall( - gas, // forward all gas - _readOnlyProxyCallee, // calling staking contract - 0x0, // start of input (calldata) - sub(calldatasize(), 4), // length of input (calldata) - 0x0, // write output over input - 0 // length of output is unknown - ) - - // copy return data to memory and *revert* - returndatacopy( - 0x0, - 0x0, - returndatasize() - ) - - revert(0, returndatasize()) - } + readOnlyProxyCallee.proxyCall( + LibProxy.RevertRule.ALWAYS_REVERT, + bytes4(0), // no custom selector + true // ignore this selector + ); } } diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 044d2d27e2..ef9d5039ae 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -18,6 +18,7 @@ pragma solidity ^0.5.9; +import "./libs/LibProxy.sol"; import "./immutable/MixinStorage.sol"; import "./interfaces/IStakingProxy.sol"; @@ -29,6 +30,8 @@ contract StakingProxy is MixinStorage { + using LibProxy for address; + /// @dev Constructor. /// @param _stakingContract Staking contract to delegate calls to. constructor(address _stakingContract, address _readOnlyProxy) @@ -46,44 +49,11 @@ contract StakingProxy is external payable { - address _stakingContract = stakingContract; - if (_stakingContract == NIL_ADDRESS) { - return; - } - - assembly { - // copy calldata to memory - calldatacopy( - 0x0, - 0x0, - calldatasize() - ) - - // delegate call into staking contract - let success := delegatecall( - gas, // forward all gas - _stakingContract, // calling staking contract - 0x0, // start of input (calldata) - calldatasize(), // length of input (calldata) - 0x0, // write output over input - 0 // length of output is unknown - ) - - // copy return data to memory - returndatacopy( - 0x0, - 0x0, - returndatasize() - ) - - // rethrow any exceptions - if iszero(success) { - revert(0, returndatasize()) - } - - // return call results - return(0, returndatasize()) - } + stakingContract.proxyCall( + LibProxy.RevertRule.REVERT_ON_ERROR, + bytes4(0), // no custom selector + false // do not ignore this selector + ); } /// @dev Attach a staking contract; future calls will be delegated to the staking contract. diff --git a/contracts/staking/contracts/src/libs/LibProxy.sol b/contracts/staking/contracts/src/libs/LibProxy.sol new file mode 100644 index 0000000000..2c1baba757 --- /dev/null +++ b/contracts/staking/contracts/src/libs/LibProxy.sol @@ -0,0 +1,109 @@ +/* + + Copyright 2019 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.5.9; + +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; +import "./LibStakingRichErrors.sol"; + + +library LibProxy { + + enum RevertRule { + REVERT_ON_ERROR, + ALWAYS_REVERT, + NEVER_REVERT + } + + /// @dev Executes a read-only call to the staking contract, via `revertDelegateCall`. + /// By routing through `revertDelegateCall` any state changes are reverted. + // solhint-disable no-complex-fallback + function proxyCall( + address destination, + RevertRule revertRule, + bytes4 customSelector, + bool ignoreSelector + ) + internal + { + if (destination == address(0)) { + LibRichErrors.rrevert( + LibStakingRichErrors.ProxyDestinationCannotBeNil() + ); + } + + assembly { + // store selector of destination function + let freeMemPtr := 0 + if gt(customSelector, 0) { + mstore(0x0, customSelector) + freeMemPtr := add(freeMemPtr, 4) + } + + // adjust the calldata offset, if we should ignore the selector + let calldataOffset := 0 + if gt(ignoreSelector, 0) { + calldataOffset := 4 + } + + // copy calldata to memory + calldatacopy( + freeMemPtr, + calldataOffset, + calldatasize() + ) + freeMemPtr := add( + freeMemPtr, + sub(calldatasize(), calldataOffset) + ) + + // delegate call into staking contract + let success := delegatecall( + gas, // forward all gas + destination, // calling staking contract + 0x0, // start of input (calldata) + freeMemPtr, // length of input (calldata) + 0x0, // write output over input + 0 // length of output is unknown + ) + + // copy return data to memory and *return* + returndatacopy( + 0x0, + 0x0, + returndatasize() + ) + + switch revertRule + case 1 { // ALWAYS_REVERT + revert(0, returndatasize()) + } + case 2 { // NEVER_REVERT + return(0, returndatasize()) + } + default {} // REVERT_ON_ERROR (handled below) + + // rethrow any exceptions + if iszero(success) { + revert(0, returndatasize()) + } + // return call results + return(0, returndatasize()) + } + } +} diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 67a195778f..92e07f30f5 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -134,6 +134,10 @@ library LibStakingRichErrors { bytes4 internal constant INVALID_STAKE_STATUS_ERROR_SELECTOR = 0xb7161acd; + // bytes4(keccak256("ProxyDestinationCannotBeNil()")) + bytes4 internal constant PROXY_DESTINATION_CANNOT_BE_NIL = + 0x01ecebea; + // solhint-disable func-name-mixedcase function MiscalculatedRewardsError( uint256 totalRewardsPaid, @@ -505,4 +509,15 @@ library LibStakingRichErrors { status ); } + + function ProxyDestinationCannotBeNil() + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + PROXY_DESTINATION_CANNOT_BE_NIL + ); + } + } diff --git a/contracts/staking/package.json b/contracts/staking/package.json index bc909078ce..aa5f3ea8fd 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|IStructs|IVaultCore|IWallet|IZrxVault|LibEIP712Hash|LibFixedMath|LibFixedMathRichErrors|LibSafeDowncast|LibSignatureValidator|LibStakingRichErrors|MixinConstants|MixinDeploymentConstants|MixinEthVault|MixinExchangeFees|MixinExchangeManager|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|MixinZrxVault|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestLibFixedMath|TestStorageLayout|ZrxVault).json" + "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStructs|IVaultCore|IWallet|IZrxVault|LibEIP712Hash|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibSignatureValidator|LibStakingRichErrors|MixinConstants|MixinDeploymentConstants|MixinEthVault|MixinExchangeFees|MixinExchangeManager|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|MixinZrxVault|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestLibFixedMath|TestStorageLayout|ZrxVault).json" }, "repository": { "type": "git", diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 354e25cf40..4c6aef1f8d 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -18,6 +18,7 @@ import * as IZrxVault from '../generated-artifacts/IZrxVault.json'; import * as LibEIP712Hash from '../generated-artifacts/LibEIP712Hash.json'; import * as LibFixedMath from '../generated-artifacts/LibFixedMath.json'; import * as LibFixedMathRichErrors from '../generated-artifacts/LibFixedMathRichErrors.json'; +import * as LibProxy from '../generated-artifacts/LibProxy.json'; import * as LibSafeDowncast from '../generated-artifacts/LibSafeDowncast.json'; import * as LibSignatureValidator from '../generated-artifacts/LibSignatureValidator.json'; import * as LibStakingRichErrors from '../generated-artifacts/LibStakingRichErrors.json'; @@ -65,6 +66,7 @@ export const artifacts = { LibEIP712Hash: LibEIP712Hash as ContractArtifact, LibFixedMath: LibFixedMath as ContractArtifact, LibFixedMathRichErrors: LibFixedMathRichErrors as ContractArtifact, + LibProxy: LibProxy as ContractArtifact, LibSafeDowncast: LibSafeDowncast as ContractArtifact, LibSignatureValidator: LibSignatureValidator as ContractArtifact, LibStakingRichErrors: LibStakingRichErrors as ContractArtifact, diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index dc37072dbe..5aaa289b29 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -16,6 +16,7 @@ export * from '../generated-wrappers/i_zrx_vault'; export * from '../generated-wrappers/lib_e_i_p712_hash'; export * from '../generated-wrappers/lib_fixed_math'; export * from '../generated-wrappers/lib_fixed_math_rich_errors'; +export * from '../generated-wrappers/lib_proxy'; export * from '../generated-wrappers/lib_safe_downcast'; export * from '../generated-wrappers/lib_signature_validator'; export * from '../generated-wrappers/lib_staking_rich_errors'; diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index 471906c580..33c7be3915 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -16,6 +16,7 @@ "generated-artifacts/LibEIP712Hash.json", "generated-artifacts/LibFixedMath.json", "generated-artifacts/LibFixedMathRichErrors.json", + "generated-artifacts/LibProxy.json", "generated-artifacts/LibSafeDowncast.json", "generated-artifacts/LibSignatureValidator.json", "generated-artifacts/LibStakingRichErrors.json",