From d0c6d9cf2dfd0c2bc44dd88d456e9f5d2ddfc009 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 4 Sep 2019 04:21:41 -0700 Subject: [PATCH] Read-Only mode in proxy --- .../staking/contracts/src/ReadOnlyProxy.sol | 107 ++++++++++++++++++ .../staking/contracts/src/StakingProxy.sol | 19 +++- .../contracts/src/immutable/MixinStorage.sol | 6 + .../src/interfaces/IStakingProxy.sol | 5 + contracts/staking/package.json | 2 +- contracts/staking/src/artifacts.ts | 2 + contracts/staking/src/wrappers.ts | 1 + contracts/staking/test/catastrophy_test.ts | 89 +++++++++++++++ .../staking/test/utils/staking_wrapper.ts | 16 +++ contracts/staking/tsconfig.json | 1 + 10 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 contracts/staking/contracts/src/ReadOnlyProxy.sol create mode 100644 contracts/staking/test/catastrophy_test.ts diff --git a/contracts/staking/contracts/src/ReadOnlyProxy.sol b/contracts/staking/contracts/src/ReadOnlyProxy.sol new file mode 100644 index 0000000000..08cc52b444 --- /dev/null +++ b/contracts/staking/contracts/src/ReadOnlyProxy.sol @@ -0,0 +1,107 @@ +/* + + 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 "./immutable/MixinStorage.sol"; + + +contract ReadOnlyProxy is + MixinStorage +{ + + /// @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()) + } + } + + /// @dev Executes a delegate call to the staking contract, if it is set. + /// This function always reverts with the return data. + 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()) + } + } +} diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index b3d283381d..044d2d27e2 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -31,11 +31,13 @@ contract StakingProxy is /// @dev Constructor. /// @param _stakingContract Staking contract to delegate calls to. - constructor(address _stakingContract) + constructor(address _stakingContract, address _readOnlyProxy) public MixinStorage() { stakingContract = _stakingContract; + readOnlyProxyCallee = _stakingContract; + readOnlyProxy = _readOnlyProxy; } /// @dev Delegates calls to the staking contract, if it is set. @@ -92,6 +94,7 @@ contract StakingProxy is onlyOwner { stakingContract = _stakingContract; + readOnlyProxyCallee = _stakingContract; emit StakingContractAttachedToProxy(_stakingContract); } @@ -104,4 +107,18 @@ contract StakingProxy is stakingContract = NIL_ADDRESS; emit StakingContractDetachedFromProxy(); } + + /// @dev Set read-only mode (state cannot be changed). + function setReadOnlyMode(bool readOnlyMode) + external + onlyOwner + { + if (readOnlyMode) { + stakingContract = readOnlyProxy; + } else { + stakingContract = readOnlyProxyCallee; + } + + emit ReadOnlyModeSet(readOnlyMode); + } } diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 6d388e100d..6a793b2de9 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -41,6 +41,12 @@ contract MixinStorage is // address of staking contract address internal stakingContract; + // address of read-only proxy + address internal readOnlyProxy; + + // address for read-only proxy to call + address internal readOnlyProxyCallee; + // mapping from Owner to Amount of Active Stake // (access using _loadAndSyncBalance or _loadUnsyncedBalance) mapping (address => IStructs.StoredBalance) internal activeStakeByOwner; diff --git a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol index 15952ebb6a..495a9035e5 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol @@ -31,6 +31,11 @@ interface IStakingProxy /* is IStaking */ /// @dev Emitted by StakingProxy when a staking contract is detached. event StakingContractDetachedFromProxy(); + /// @dev Emitted by StakingProxy when read-only mode is set. + event ReadOnlyModeSet( + bool readOnlyMode + ); + /// @dev Delegates calls to the staking contract, if it is set. // solhint-disable no-complex-fallback function () diff --git a/contracts/staking/package.json b/contracts/staking/package.json index d35f278cde..bc909078ce 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|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestLibFixedMath|TestStorageLayout|ZrxVault).json" + "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" }, "repository": { "type": "git", diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 538ccaa2f1..354e25cf40 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -36,6 +36,7 @@ import * as MixinStakingPoolRewardVault from '../generated-artifacts/MixinStakin import * as MixinStorage from '../generated-artifacts/MixinStorage.json'; import * as MixinVaultCore from '../generated-artifacts/MixinVaultCore.json'; import * as MixinZrxVault from '../generated-artifacts/MixinZrxVault.json'; +import * as ReadOnlyProxy from '../generated-artifacts/ReadOnlyProxy.json'; import * as Staking from '../generated-artifacts/Staking.json'; import * as StakingPoolRewardVault from '../generated-artifacts/StakingPoolRewardVault.json'; import * as StakingProxy from '../generated-artifacts/StakingProxy.json'; @@ -44,6 +45,7 @@ import * as TestLibFixedMath from '../generated-artifacts/TestLibFixedMath.json' import * as TestStorageLayout from '../generated-artifacts/TestStorageLayout.json'; import * as ZrxVault from '../generated-artifacts/ZrxVault.json'; export const artifacts = { + ReadOnlyProxy: ReadOnlyProxy as ContractArtifact, Staking: Staking as ContractArtifact, StakingProxy: StakingProxy as ContractArtifact, MixinExchangeFees: MixinExchangeFees as ContractArtifact, diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index 1f1d22eb4a..dc37072dbe 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -34,6 +34,7 @@ export * from '../generated-wrappers/mixin_staking_pool_rewards'; export * from '../generated-wrappers/mixin_storage'; export * from '../generated-wrappers/mixin_vault_core'; export * from '../generated-wrappers/mixin_zrx_vault'; +export * from '../generated-wrappers/read_only_proxy'; export * from '../generated-wrappers/staking'; export * from '../generated-wrappers/staking_pool_reward_vault'; export * from '../generated-wrappers/staking_proxy'; diff --git a/contracts/staking/test/catastrophy_test.ts b/contracts/staking/test/catastrophy_test.ts new file mode 100644 index 0000000000..969f843b2f --- /dev/null +++ b/contracts/staking/test/catastrophy_test.ts @@ -0,0 +1,89 @@ +import { ERC20ProxyContract, ERC20Wrapper } from '@0x/contracts-asset-proxy'; +import { DummyERC20TokenContract } from '@0x/contracts-erc20'; +import { blockchainTests, describe, expect, provider, web3Wrapper } from '@0x/contracts-test-utils'; +import { BigNumber } from '@0x/utils'; +import { LogWithDecodedArgs } from 'ethereum-types'; +import * as _ from 'lodash'; + +import { StakingProxyReadOnlyModeSetEventArgs } from '../src'; + +import { StakingWrapper } from './utils/staking_wrapper'; + +// tslint:disable:no-unnecessary-type-assertion +blockchainTests.resets('Catastrophy Tests', () => { + // constants + const ZRX_TOKEN_DECIMALS = new BigNumber(18); + const ZERO = new BigNumber(0); + // tokens & addresses + let accounts: string[]; + let owner: string; + let actors: string[]; + let zrxTokenContract: DummyERC20TokenContract; + let erc20ProxyContract: ERC20ProxyContract; + // wrappers + let stakingWrapper: StakingWrapper; + let erc20Wrapper: ERC20Wrapper; + // tests + before(async () => { + // create accounts + accounts = await web3Wrapper.getAvailableAddressesAsync(); + owner = accounts[0]; + actors = accounts.slice(2, 5); + // deploy erc20 proxy + erc20Wrapper = new ERC20Wrapper(provider, accounts, owner); + erc20ProxyContract = await erc20Wrapper.deployProxyAsync(); + // deploy zrx token + [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS); + await erc20Wrapper.setBalancesAndAllowancesAsync(); + // deploy staking contracts + stakingWrapper = new StakingWrapper(provider, owner, erc20ProxyContract, zrxTokenContract, accounts); + await stakingWrapper.deployAndConfigureContractsAsync(); + }); + + describe('Read-Only Mode', () => { + it('should be able to change state by default', async () => { + // stake some zrx and assert the balance + const amountToStake = StakingWrapper.toBaseUnitAmount(10); + await stakingWrapper.stakeAsync(actors[0], amountToStake); + const activeStakeBalance = await stakingWrapper.getActiveStakeAsync(actors[0]); + expect(activeStakeBalance.currentEpochBalance).to.be.bignumber.equal(amountToStake); + }); + it('should not change state when in read-only mode', async () => { + // set to read-only mode + await stakingWrapper.setReadOnlyModeAsync(true); + // try to stake + const amountToStake = StakingWrapper.toBaseUnitAmount(10); + await stakingWrapper.stakeAsync(actors[0], amountToStake); + const activeStakeBalance = await stakingWrapper.getActiveStakeAsync(actors[0]); + expect(activeStakeBalance.currentEpochBalance).to.be.bignumber.equal(ZERO); + }); + it('should read values correctly when in read-only mode', async () => { + // stake some zrx + const amountToStake = StakingWrapper.toBaseUnitAmount(10); + await stakingWrapper.stakeAsync(actors[0], amountToStake); + // set to read-only mode + await stakingWrapper.setReadOnlyModeAsync(true); + // read stake balance in read-only mode + const activeStakeBalanceReadOnly = await stakingWrapper.getActiveStakeAsync(actors[0]); + expect(activeStakeBalanceReadOnly.currentEpochBalance).to.be.bignumber.equal(amountToStake); + }); + it('should exit read-only mode', async () => { + // set to read-only mode + await stakingWrapper.setReadOnlyModeAsync(true); + await stakingWrapper.setReadOnlyModeAsync(false); + // try to stake + const amountToStake = StakingWrapper.toBaseUnitAmount(10); + await stakingWrapper.stakeAsync(actors[0], amountToStake); + const activeStakeBalance = await stakingWrapper.getActiveStakeAsync(actors[0]); + expect(activeStakeBalance.currentEpochBalance).to.be.bignumber.equal(amountToStake); + }); + it('should emit event when setting read-only mode', async () => { + // set to read-only mode + const txReceipt = await stakingWrapper.setReadOnlyModeAsync(true); + expect(txReceipt.logs.length).to.be.equal(1); + const trueLog = txReceipt.logs[0] as LogWithDecodedArgs; + expect(trueLog.args.readOnlyMode).to.be.true(); + }); + }); +}); +// tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/staking/test/utils/staking_wrapper.ts b/contracts/staking/test/utils/staking_wrapper.ts index 00f9f98837..68e2d349af 100644 --- a/contracts/staking/test/utils/staking_wrapper.ts +++ b/contracts/staking/test/utils/staking_wrapper.ts @@ -10,6 +10,7 @@ import * as _ from 'lodash'; import { artifacts, EthVaultContract, + ReadOnlyProxyContract, StakingContract, StakingPoolRewardVaultContract, StakingProxyContract, @@ -33,6 +34,7 @@ export class StakingWrapper { private _zrxVaultContractIfExists?: ZrxVaultContract; private _ethVaultContractIfExists?: EthVaultContract; private _rewardVaultContractIfExists?: StakingPoolRewardVaultContract; + private _readOnlyProxyContractIfExists?: ReadOnlyProxyContract; public static toBaseUnitAmount(amount: BigNumber | number): BigNumber { const decimals = 18; const amountAsBigNumber = typeof amount === 'number' ? new BigNumber(amount) : amount; @@ -97,6 +99,13 @@ export class StakingWrapper { return this._rewardVaultContractIfExists as StakingPoolRewardVaultContract; } public async deployAndConfigureContractsAsync(): Promise { + // deploy read-only proxy + this._readOnlyProxyContractIfExists = await ReadOnlyProxyContract.deployFrom0xArtifactAsync( + artifacts.ReadOnlyProxy, + this._provider, + txDefaults, + artifacts, + ); // deploy zrx vault this._zrxVaultContractIfExists = await ZrxVaultContract.deployFrom0xArtifactAsync( artifacts.ZrxVault, @@ -142,6 +151,7 @@ export class StakingWrapper { txDefaults, artifacts, this._stakingContractIfExists.address, + this._readOnlyProxyContractIfExists.address, ); // set staking proxy contract in zrx vault await this._zrxVaultContractIfExists.setStakingContract.awaitTransactionSuccessAsync( @@ -176,6 +186,12 @@ export class StakingWrapper { await this._web3Wrapper.sendTransactionAsync(setStakingPoolRewardVaultTxData), ); } + public async setReadOnlyModeAsync(readOnlyMode: boolean): Promise { + const txReceipt = await this.getStakingProxyContract().setReadOnlyMode.awaitTransactionSuccessAsync( + readOnlyMode, + ); + return txReceipt; + } public async getEthBalanceAsync(owner: string): Promise { const balance = this._web3Wrapper.getBalanceInWeiAsync(owner); return balance; diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index 7d94b6ecc4..471906c580 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -34,6 +34,7 @@ "generated-artifacts/MixinStorage.json", "generated-artifacts/MixinVaultCore.json", "generated-artifacts/MixinZrxVault.json", + "generated-artifacts/ReadOnlyProxy.json", "generated-artifacts/Staking.json", "generated-artifacts/StakingPoolRewardVault.json", "generated-artifacts/StakingProxy.json",