@0x/contract-staking: Refactor out contract attach logic so it's shared between the constructor and attachStakingContract().

`@0x/contract-staking`: Introduce migration tests to test the `init()` pattern.
This commit is contained in:
Lawrence Forman
2019-09-06 18:32:20 -04:00
parent 76c5517739
commit 7d50117903
8 changed files with 245 additions and 12 deletions

View File

@@ -44,10 +44,9 @@ contract StakingProxy is
public
MixinStorage()
{
stakingContract = _stakingContract;
readOnlyProxyCallee = _stakingContract;
readOnlyProxy = _readOnlyProxy;
wethAssetProxy = IAssetProxy(_wethProxyAddress);
_attachStakingContract(_stakingContract);
}
/// @dev Delegates calls to the staking contract, if it is set.
@@ -69,15 +68,7 @@ contract StakingProxy is
external
onlyOwner
{
stakingContract = _stakingContract;
readOnlyProxyCallee = _stakingContract;
// Call `init()` on the staking contract to initialize storage.
(bool didInitSucceed, bytes memory initReturnData) =
stakingContract.delegatecall(abi.encode(IStorageInit(0).init.selector));
if (!didInitSucceed) {
assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) }
}
emit StakingContractAttachedToProxy(_stakingContract);
_attachStakingContract(_stakingContract);
}
/// @dev Detach the current staking contract.
@@ -103,4 +94,19 @@ contract StakingProxy is
emit ReadOnlyModeSet(readOnlyMode);
}
/// @dev Attach a staking contract; future calls will be delegated to the staking contract.
/// @param _stakingContract Address of staking contract.
function _attachStakingContract(address _stakingContract)
private
{
stakingContract = readOnlyProxyCallee = _stakingContract;
// Call `init()` on the staking contract to initialize storage.
(bool didInitSucceed, bytes memory initReturnData) =
_stakingContract.delegatecall(abi.encode(IStorageInit(0).init.selector));
if (!didInitSucceed) {
assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) }
}
emit StakingContractAttachedToProxy(_stakingContract);
}
}

View File

@@ -0,0 +1,59 @@
/*
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;
pragma experimental ABIEncoderV2;
import "@0x/contracts-utils/contracts/src/Ownable.sol";
import "../src/immutable/MixinStorage.sol";
import "../src/immutable/MixinHyperParameters.sol";
contract TestInitTarget is
Ownable,
MixinStorage,
MixinHyperParameters
{
// 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.
address public constant SHOULD_REVERT_ADDRESS = 0x5ed6A38c6bEcEd15b0AB58566b6fD7A00463d2F7;
// `msg.sender` of the last `init()` call.
address private _initSender = address(0);
// `address(this)` of the last `init()` call.
address private _initThisAddress = address(0);
function init() external {
if (SHOULD_REVERT_ADDRESS.balance != 0) {
revert("WHOOPSIE!");
}
_initSender = msg.sender;
_initThisAddress = address(this);
}
function getInitState()
external
view
returns (
address initSender,
address initThisAddress
)
{
initSender = _initSender;
initThisAddress = _initThisAddress;
}
}

View File

@@ -0,0 +1,33 @@
/*
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;
pragma experimental ABIEncoderV2;
import "../src/StakingProxy.sol";
contract TestStakingProxy is
StakingProxy
{
// solhint-disable-next-line no-empty-blocks
constructor(address _stakingContract) public StakingProxy(_stakingContract) {}
function getAttachedContract() external view returns (address) {
return stakingContract;
}
}

View File

@@ -40,10 +40,12 @@ import * as Staking from '../generated-artifacts/Staking.json';
import * as StakingPoolRewardVault from '../generated-artifacts/StakingPoolRewardVault.json';
import * as StakingProxy from '../generated-artifacts/StakingProxy.json';
import * as TestCobbDouglas from '../generated-artifacts/TestCobbDouglas.json';
import * as TestInitTarget from '../generated-artifacts/TestInitTarget.json';
import * as TestLibFixedMath from '../generated-artifacts/TestLibFixedMath.json';
import * as TestProtocolFees from '../generated-artifacts/TestProtocolFees.json';
import * as TestProtocolFeesERC20Proxy from '../generated-artifacts/TestProtocolFeesERC20Proxy.json';
import * as TestStaking from '../generated-artifacts/TestStaking.json';
import * as TestStakingProxy from '../generated-artifacts/TestStakingProxy.json';
import * as TestStorageLayout from '../generated-artifacts/TestStorageLayout.json';
import * as ZrxVault from '../generated-artifacts/ZrxVault.json';
export const artifacts = {
@@ -83,9 +85,11 @@ export const artifacts = {
StakingPoolRewardVault: StakingPoolRewardVault as ContractArtifact,
ZrxVault: ZrxVault as ContractArtifact,
TestCobbDouglas: TestCobbDouglas as ContractArtifact,
TestInitTarget: TestInitTarget as ContractArtifact,
TestLibFixedMath: TestLibFixedMath as ContractArtifact,
TestProtocolFees: TestProtocolFees as ContractArtifact,
TestProtocolFeesERC20Proxy: TestProtocolFeesERC20Proxy as ContractArtifact,
TestStaking: TestStaking as ContractArtifact,
TestStakingProxy: TestStakingProxy as ContractArtifact,
TestStorageLayout: TestStorageLayout as ContractArtifact,
};

View File

@@ -38,9 +38,11 @@ export * from '../generated-wrappers/staking';
export * from '../generated-wrappers/staking_pool_reward_vault';
export * from '../generated-wrappers/staking_proxy';
export * from '../generated-wrappers/test_cobb_douglas';
export * from '../generated-wrappers/test_init_target';
export * from '../generated-wrappers/test_lib_fixed_math';
export * from '../generated-wrappers/test_protocol_fees';
export * from '../generated-wrappers/test_protocol_fees_erc20_proxy';
export * from '../generated-wrappers/test_staking';
export * from '../generated-wrappers/test_staking_proxy';
export * from '../generated-wrappers/test_storage_layout';
export * from '../generated-wrappers/zrx_vault';

View File

@@ -1,7 +1,6 @@
import { blockchainTests, constants, expect, filterLogsToArguments, Numberish } from '@0x/contracts-test-utils';
import { StakingRevertErrors } from '@0x/order-utils';
import { BigNumber, OwnableRevertErrors } from '@0x/utils';
import * as _ from 'lodash';
import { artifacts, IStakingEventsTunedEventArgs, MixinHyperParametersContract } from '../src/';

View File

@@ -0,0 +1,128 @@
import { blockchainTests, constants, expect, hexRandom } from '@0x/contracts-test-utils';
import { BigNumber, OwnableRevertErrors, StringRevertError } from '@0x/utils';
import { artifacts, StakingContract, TestInitTargetContract, TestStakingProxyContract } from '../src/';
blockchainTests('Migration tests', env => {
let ownerAddress: string;
let notOwnerAddress: string;
before(async () => {
[ownerAddress, notOwnerAddress] = await env.getAccountAddressesAsync();
});
describe('StakingProxy', () => {
const REVERT_ERROR = new StringRevertError('WHOOPSIE!');
let initTargetContract: TestInitTargetContract;
async function deployStakingProxyAsync(stakingContractAddress?: string): Promise<TestStakingProxyContract> {
return TestStakingProxyContract.deployFrom0xArtifactAsync(
artifacts.TestStakingProxy,
env.provider,
env.txDefaults,
artifacts,
stakingContractAddress || constants.NULL_ADDRESS,
);
}
before(async () => {
[ownerAddress, notOwnerAddress] = await env.getAccountAddressesAsync();
initTargetContract = await TestInitTargetContract.deployFrom0xArtifactAsync(
artifacts.TestInitTarget,
env.provider,
env.txDefaults,
artifacts,
);
});
function randomAddress(): string {
return hexRandom(constants.ADDRESS_LENGTH);
}
async function enableInitRevertsAsync(): Promise<void> {
const revertAddress = await initTargetContract.SHOULD_REVERT_ADDRESS.callAsync();
// Deposit some ether into `revertAddress` to signal `initTargetContract`
// to fail.
await env.web3Wrapper.awaitTransactionMinedAsync(
await env.web3Wrapper.sendTransactionAsync({
...env.txDefaults,
from: ownerAddress,
to: revertAddress,
data: '0x',
value: new BigNumber(1),
}),
);
}
async function assertInitStateAsync(proxyContract: TestStakingProxyContract): Promise<void> {
const [senderAddress, thisAddress] = await initTargetContract.getInitState.callAsync({
to: proxyContract.address,
});
expect(senderAddress).to.eq(ownerAddress);
expect(thisAddress).to.eq(proxyContract.address);
const attachedAddress = await proxyContract.getAttachedContract.callAsync();
expect(attachedAddress).to.eq(initTargetContract.address);
}
blockchainTests.resets('StakingProxy constructor', async () => {
it('calls init() and attaches the contract', async () => {
const proxyContract = await deployStakingProxyAsync(initTargetContract.address);
await assertInitStateAsync(proxyContract);
});
it('reverts if init() reverts', async () => {
await enableInitRevertsAsync();
const tx = deployStakingProxyAsync(initTargetContract.address);
return expect(tx).to.revertWith(REVERT_ERROR);
});
});
blockchainTests.resets('attachStakingContract()', async () => {
let proxyContract: TestStakingProxyContract;
before(async () => {
proxyContract = await deployStakingProxyAsync();
});
it('throws if not called by owner', 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 () => {
let stakingContract: StakingContract;
before(async () => {
stakingContract = await StakingContract.deployFrom0xArtifactAsync(
artifacts.Staking,
env.provider,
env.txDefaults,
artifacts,
);
});
it('throws if not called by owner', async () => {
const tx = stakingContract.init.awaitTransactionSuccessAsync({ from: notOwnerAddress });
const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwnerAddress, ownerAddress);
return expect(tx).to.revertWith(expectedError);
});
});
});
// tslint:enable:no-unnecessary-type-assertion

View File

@@ -38,10 +38,12 @@
"generated-artifacts/StakingPoolRewardVault.json",
"generated-artifacts/StakingProxy.json",
"generated-artifacts/TestCobbDouglas.json",
"generated-artifacts/TestInitTarget.json",
"generated-artifacts/TestLibFixedMath.json",
"generated-artifacts/TestProtocolFees.json",
"generated-artifacts/TestProtocolFeesERC20Proxy.json",
"generated-artifacts/TestStaking.json",
"generated-artifacts/TestStakingProxy.json",
"generated-artifacts/TestStorageLayout.json",
"generated-artifacts/ZrxVault.json"
],