From 7bfbf0ad3ae6b903cc55950edd23bdb76c75b7d0 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 Oct 2019 15:36:20 -0700 Subject: [PATCH] Staking Proxy unit tests + Staking Proxy fallback reverts if no staking contract is attached --- .../staking/contracts/src/StakingProxy.sol | 11 +- .../contracts/test/TestProxyDestination.sol | 82 +++++ .../contracts/test/TestStakingProxyUnit.sol | 62 ++++ contracts/staking/src/artifacts.ts | 4 + contracts/staking/src/wrappers.ts | 2 + .../test/unit_tests/staking_proxy_test.ts | 281 ++++++++++++++++++ contracts/staking/tsconfig.json | 2 + 7 files changed, 443 insertions(+), 1 deletion(-) create mode 100644 contracts/staking/contracts/test/TestProxyDestination.sol create mode 100644 contracts/staking/contracts/test/TestStakingProxyUnit.sol create mode 100644 contracts/staking/test/unit_tests/staking_proxy_test.ts diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 78abfa5ded..1bb77caeea 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -54,8 +54,16 @@ contract StakingProxy is external payable { + // Sanity check that we have a staking contract to call + address stakingContract_ = stakingContract; + if (stakingContract_ == NIL_ADDRESS) { + LibRichErrors.rrevert( + LibStakingRichErrors.ProxyDestinationCannotBeNilError() + ); + } + // Call the staking contract with the provided calldata. - (bool success, bytes memory returnData) = stakingContract.delegatecall(msg.data); + (bool success, bytes memory returnData) = stakingContract_.delegatecall(msg.data); // Revert on failure or return on success. assembly { @@ -186,6 +194,7 @@ contract StakingProxy is (bool didInitSucceed, bytes memory initReturnData) = stakingContract.delegatecall( abi.encodeWithSelector(IStorageInit(0).init.selector) ); + if (!didInitSucceed) { assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) diff --git a/contracts/staking/contracts/test/TestProxyDestination.sol b/contracts/staking/contracts/test/TestProxyDestination.sol new file mode 100644 index 0000000000..11dc41714b --- /dev/null +++ b/contracts/staking/contracts/test/TestProxyDestination.sol @@ -0,0 +1,82 @@ +/* + + 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/Staking.sol"; + + +contract TestProxyDestination is + Staking +{ + // Init will revert if this flag is set to `true` + bool public initFailFlag; + + /// @dev Emitted when `init` is called + event InitCalled( + bool initCalled + ); + + /// @dev returns the input string + function echo(string calldata val) + external + returns (string memory) + { + return val; + } + + /// @dev Just a function that'll do some math on input + function doMath(uint256 a, uint256 b) + external + returns (uint256 sum, uint256 difference) + { + return ( + a + b, + a - b + ); + } + + /// @dev reverts with "Goodbye, World!" + function die() + external + { + revert("Goodbye, World!"); + } + + /// @dev Called when attached to the StakingProxy. + /// Reverts if `initFailFlag` is set, otherwise + /// sets storage params and emits `InitCalled`. + function init() + public + { + if (initFailFlag) { + revert("INIT_FAIL_FLAG_SET"); + } + + // Set params such that they'll pass `StakingProxy.assertValidStorageParams` + epochDurationInSeconds = 5 days; + cobbDouglasAlphaNumerator = 1; + cobbDouglasAlphaDenominator = 1; + rewardDelegatedStakeWeight = PPM_DENOMINATOR; + minimumPoolStake = 100; + + // Emit event to notify that `init` was called + emit InitCalled(true); + } +} \ No newline at end of file diff --git a/contracts/staking/contracts/test/TestStakingProxyUnit.sol b/contracts/staking/contracts/test/TestStakingProxyUnit.sol new file mode 100644 index 0000000000..523ddc544b --- /dev/null +++ b/contracts/staking/contracts/test/TestStakingProxyUnit.sol @@ -0,0 +1,62 @@ +/* + + 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 TestStakingProxyUnit is + StakingProxy +{ + // Storage Params - these are tested by StakingProxy.assertValidStorageParams. + struct TestStorageParams { + uint256 epochDurationInSeconds; + uint32 cobbDouglasAlphaNumerator; + uint32 cobbDouglasAlphaDenominator; + uint32 rewardDelegatedStakeWeight; + uint256 minimumPoolStake; + } + + // If this is set then the `init` call will revert in the `TestProxyDestination` contract + bool public initFailFlag; + + // solhint-disable no-empty-blocks + constructor(address _stakingContract) + public + StakingProxy( _stakingContract) + {} + + // Setters to modify the + function setInitFailFlag() + external + { + initFailFlag = true; + } + + /// @dev Sets storage params with test values + function setTestStorageParams(TestStorageParams calldata params) + external + { + epochDurationInSeconds = params.epochDurationInSeconds; + cobbDouglasAlphaNumerator = params.cobbDouglasAlphaNumerator; + cobbDouglasAlphaDenominator = params.cobbDouglasAlphaDenominator; + rewardDelegatedStakeWeight = params.rewardDelegatedStakeWeight; + minimumPoolStake = params.minimumPoolStake; + } +} diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index f5674e3b11..db17417685 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -49,9 +49,11 @@ import * as TestMixinStakeBalances from '../generated-artifacts/TestMixinStakeBa import * as TestMixinStakeStorage from '../generated-artifacts/TestMixinStakeStorage.json'; import * as TestMixinStakingPool from '../generated-artifacts/TestMixinStakingPool.json'; import * as TestProtocolFees from '../generated-artifacts/TestProtocolFees.json'; +import * as TestProxyDestination from '../generated-artifacts/TestProxyDestination.json'; import * as TestStaking from '../generated-artifacts/TestStaking.json'; import * as TestStakingNoWETH from '../generated-artifacts/TestStakingNoWETH.json'; import * as TestStakingProxy from '../generated-artifacts/TestStakingProxy.json'; +import * as TestStakingProxyUnit from '../generated-artifacts/TestStakingProxyUnit.json'; import * as TestStorageLayoutAndConstants from '../generated-artifacts/TestStorageLayoutAndConstants.json'; import * as ZrxVault from '../generated-artifacts/ZrxVault.json'; export const artifacts = { @@ -100,8 +102,10 @@ export const artifacts = { TestMixinStakeStorage: TestMixinStakeStorage as ContractArtifact, TestMixinStakingPool: TestMixinStakingPool as ContractArtifact, TestProtocolFees: TestProtocolFees as ContractArtifact, + TestProxyDestination: TestProxyDestination as ContractArtifact, TestStaking: TestStaking as ContractArtifact, TestStakingNoWETH: TestStakingNoWETH as ContractArtifact, TestStakingProxy: TestStakingProxy as ContractArtifact, + TestStakingProxyUnit: TestStakingProxyUnit as ContractArtifact, TestStorageLayoutAndConstants: TestStorageLayoutAndConstants as ContractArtifact, }; diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index 3e2424b760..df1b36a8ab 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -47,8 +47,10 @@ export * from '../generated-wrappers/test_mixin_stake_balances'; export * from '../generated-wrappers/test_mixin_stake_storage'; export * from '../generated-wrappers/test_mixin_staking_pool'; export * from '../generated-wrappers/test_protocol_fees'; +export * from '../generated-wrappers/test_proxy_destination'; export * from '../generated-wrappers/test_staking'; export * from '../generated-wrappers/test_staking_no_w_e_t_h'; export * from '../generated-wrappers/test_staking_proxy'; +export * from '../generated-wrappers/test_staking_proxy_unit'; export * from '../generated-wrappers/test_storage_layout_and_constants'; export * from '../generated-wrappers/zrx_vault'; diff --git a/contracts/staking/test/unit_tests/staking_proxy_test.ts b/contracts/staking/test/unit_tests/staking_proxy_test.ts new file mode 100644 index 0000000000..4def924da9 --- /dev/null +++ b/contracts/staking/test/unit_tests/staking_proxy_test.ts @@ -0,0 +1,281 @@ +import { blockchainTests, constants, expect } from '@0x/contracts-test-utils'; +import { StakingRevertErrors } from '@0x/order-utils'; +import { AuthorizableRevertErrors, BigNumber } from '@0x/utils'; +import * as _ from 'lodash'; + +import { + artifacts, + StakingProxyStakingContractAttachedToProxyEventArgs, + TestProxyDestinationContract, + TestProxyDestinationInitCalledEventArgs, + TestStakingProxyUnitContract, +} from '../../src'; + +import { constants as stakingConstants } from '../utils/constants'; + +blockchainTests.resets('StakingProxy unit tests', env => { + const testString = 'Hello, World!'; + const testRevertString = 'Goodbye, World!'; + let accounts: string[]; + let owner: string; + let authorizedAddress: string; + let notAuthorizedAddresses: string[]; + let testProxyContract: TestStakingProxyUnitContract; + let testContractViaProxy: TestProxyDestinationContract; + let testContract: TestProxyDestinationContract; + let testContract2: TestProxyDestinationContract; + + before(async () => { + // Create accounts + accounts = await env.getAccountAddressesAsync(); + [owner, authorizedAddress, ...notAuthorizedAddresses] = accounts; + + // Deploy contracts + testContract = await TestProxyDestinationContract.deployFrom0xArtifactAsync( + artifacts.TestProxyDestination, + env.provider, + env.txDefaults, + artifacts, + ); + testContract2 = await TestProxyDestinationContract.deployFrom0xArtifactAsync( + artifacts.TestProxyDestination, + env.provider, + env.txDefaults, + artifacts, + ); + testProxyContract = await TestStakingProxyUnitContract.deployFrom0xArtifactAsync( + artifacts.TestStakingProxyUnit, + env.provider, + env.txDefaults, + artifacts, + testContract.address, + ); + const logDecoderDependencies = _.mapValues(artifacts, v => v.compilerOutput.abi); + testContractViaProxy = new TestProxyDestinationContract( + testProxyContract.address, + env.provider, + env.txDefaults, + logDecoderDependencies, + ); + + // Add authorized address to Staking Proxy + await testProxyContract.addAuthorizedAddress.sendTransactionAsync(authorizedAddress, { from: owner }); + }); + + describe('Fallback function', () => { + it('should pass back the return value of the destination contract', async () => { + const returnValue = await testContractViaProxy.echo.callAsync(testString); + expect(returnValue).to.equal(testString); + }); + + it('should revert with correct value when destination reverts', async () => { + return expect(testContractViaProxy.die.callAsync()).to.revertWith(testRevertString); + }); + + it('should revert if no staking contract is attached', async () => { + await testProxyContract.detachStakingContract.awaitTransactionSuccessAsync({ from: authorizedAddress }); + const expectedError = new StakingRevertErrors.ProxyDestinationCannotBeNilError(); + const tx = testContractViaProxy.echo.callAsync(testString); + return expect(tx).to.revertWith(expectedError); + }); + }); + + describe('attachStakingContract', () => { + it('should successfully attaching a new staking contract', async () => { + // Cache existing staking contract and attach a new one + const initStakingContractAddress = await testProxyContract.stakingContract.callAsync(); + const txReceipt = await testProxyContract.attachStakingContract.awaitTransactionSuccessAsync( + testContract2.address, + { from: authorizedAddress }, + ); + + // Validate `ContractAttachedToProxy` event + expect(txReceipt.logs.length).to.be.gte(1); + const contractAttachedEvent: StakingProxyStakingContractAttachedToProxyEventArgs = (txReceipt + .logs[0] as any).args; + expect(contractAttachedEvent.newStakingContractAddress).to.equal(testContract2.address); + + // Check that `init` was called on destination contract + expect(txReceipt.logs.length).to.be.gte(2); + const initCalledEvent: TestProxyDestinationInitCalledEventArgs = (txReceipt.logs[1] as any).args; + expect(initCalledEvent.initCalled).to.be.true(); + + // Validate new staking contract address + const finalStakingContractAddress = await testProxyContract.stakingContract.callAsync(); + expect(finalStakingContractAddress).to.be.equal(testContract2.address); + expect(finalStakingContractAddress).to.not.equal(initStakingContractAddress); + }); + + it('should revert if call to `init` on new staking contract fails', async () => { + await testProxyContract.setInitFailFlag.awaitTransactionSuccessAsync(); + const tx = testProxyContract.attachStakingContract.awaitTransactionSuccessAsync(testContract2.address, { + from: authorizedAddress, + }); + const expectedError = 'INIT_FAIL_FLAG_SET'; + return expect(tx).to.revertWith(expectedError); + }); + + it('should revert if called by unauthorized address', async () => { + const tx = testProxyContract.attachStakingContract.awaitTransactionSuccessAsync(testContract2.address, { + from: notAuthorizedAddresses[0], + }); + const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(notAuthorizedAddresses[0]); + return expect(tx).to.revertWith(expectedError); + }); + }); + + describe('detachStakingContract', () => { + it('should detach staking contract', async () => { + // Cache existing staking contract and attach a new one + const initStakingContractAddress = await testProxyContract.stakingContract.callAsync(); + const txReceipt = await testProxyContract.detachStakingContract.awaitTransactionSuccessAsync({ + from: authorizedAddress, + }); + + // Validate that event was emitted + expect(txReceipt.logs.length).to.eq(1); + + // Validate staking contract address was unset + const finalStakingContractAddress = await testProxyContract.stakingContract.callAsync(); + expect(finalStakingContractAddress).to.be.equal(stakingConstants.NIL_ADDRESS); + expect(finalStakingContractAddress).to.not.equal(initStakingContractAddress); + }); + + it('should revert if called by unauthorized address', async () => { + const tx = testProxyContract.detachStakingContract.awaitTransactionSuccessAsync({ + from: notAuthorizedAddresses[0], + }); + const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(notAuthorizedAddresses[0]); + return expect(tx).to.revertWith(expectedError); + }); + }); + + describe('batchExecute', () => { + it('should execute no-op if no calls to make', async () => { + await testProxyContract.batchExecute.awaitTransactionSuccessAsync([]); + }); + + it('should call one function and return the output', async () => { + const calls = [testContract.echo.getABIEncodedTransactionData(testString)]; + const rawResults = await testProxyContract.batchExecute.callAsync(calls); + expect(rawResults.length).to.equal(1); + const returnValues = [testContract.echo.getABIDecodedReturnData(rawResults[0])]; + expect(returnValues[0]).to.equal(testString); + }); + + it('should call multiple functions and return their outputs', async () => { + const calls = [ + testContract.echo.getABIEncodedTransactionData(testString), + testContract.doMath.getABIEncodedTransactionData(new BigNumber(2), new BigNumber(1)), + ]; + const rawResults = await testProxyContract.batchExecute.callAsync(calls); + expect(rawResults.length).to.equal(2); + const returnValues = [ + testContract.echo.getABIDecodedReturnData(rawResults[0]), + testContract.doMath.getABIDecodedReturnData(rawResults[1]), + ]; + expect(returnValues[0]).to.equal(testString); + expect(returnValues[1][0]).to.bignumber.equal(new BigNumber(3)); + expect(returnValues[1][1]).to.bignumber.equal(new BigNumber(1)); + }); + + it('should revert if a call reverts', async () => { + const calls = [ + testContract.echo.getABIEncodedTransactionData(testString), + testContract.die.getABIEncodedTransactionData(), + testContract.doMath.getABIEncodedTransactionData(new BigNumber(2), new BigNumber(1)), + ]; + const tx = testProxyContract.batchExecute.callAsync(calls); + const expectedError = 'Goodbye, World!'; + return expect(tx).to.revertWith(expectedError); + }); + }); + + describe('assertValidStorageParams', () => { + const validStorageParams = { + epochDurationInSeconds: new BigNumber(5 * 24 * 60 * 60), // 5 days + cobbDouglasAlphaNumerator: new BigNumber(1), + cobbDouglasAlphaDenominator: new BigNumber(1), + rewardDelegatedStakeWeight: constants.PPM_DENOMINATOR, + minimumPoolStake: new BigNumber(100), + }; + it('should not revert if all storage params are valid', async () => { + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(validStorageParams); + await testProxyContract.assertValidStorageParams.callAsync(); + }); + it('should revert if `epochDurationInSeconds` is less than 5 days', async () => { + const invalidStorageParams = { + ...validStorageParams, + epochDurationInSeconds: new BigNumber(0), + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidEpochDuration, + ); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert if `epochDurationInSeconds` is greater than 30 days', async () => { + const invalidStorageParams = { + ...validStorageParams, + epochDurationInSeconds: new BigNumber(31 * 24 * 60 * 60), // 31 days + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidEpochDuration, + ); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert if `cobbDouglasAlphaNumerator` is greater than `cobbDouglasAlphaDenominator`', async () => { + const invalidStorageParams = { + ...validStorageParams, + cobbDouglasAlphaNumerator: new BigNumber(2), + cobbDouglasAlphaDenominator: new BigNumber(1), + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidCobbDouglasAlpha, + ); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert if `cobbDouglasAlphaDenominator` equals zero', async () => { + const invalidStorageParams = { + ...validStorageParams, + cobbDouglasAlphaDenominator: new BigNumber(0), + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidCobbDouglasAlpha, + ); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert if `rewardDelegatedStakeWeight` is greater than PPM_DENOMINATOR', async () => { + const invalidStorageParams = { + ...validStorageParams, + rewardDelegatedStakeWeight: new BigNumber(constants.PPM_DENOMINATOR + 1), + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidRewardDelegatedStakeWeight, + ); + return expect(tx).to.revertWith(expectedError); + }); + it('should revert if `minimumPoolStake` is less than two', async () => { + const invalidStorageParams = { + ...validStorageParams, + minimumPoolStake: new BigNumber(1), + }; + await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams); + const tx = testProxyContract.assertValidStorageParams.callAsync(); + const expectedError = new StakingRevertErrors.InvalidParamValueError( + StakingRevertErrors.InvalidParamValueErrorCodes.InvalidMinimumPoolStake, + ); + return expect(tx).to.revertWith(expectedError); + }); + }); +}); +// tslint:disable: max-file-line-count diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index f230efb194..3383d877ae 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -47,9 +47,11 @@ "generated-artifacts/TestMixinStakeStorage.json", "generated-artifacts/TestMixinStakingPool.json", "generated-artifacts/TestProtocolFees.json", + "generated-artifacts/TestProxyDestination.json", "generated-artifacts/TestStaking.json", "generated-artifacts/TestStakingNoWETH.json", "generated-artifacts/TestStakingProxy.json", + "generated-artifacts/TestStakingProxyUnit.json", "generated-artifacts/TestStorageLayoutAndConstants.json", "generated-artifacts/ZrxVault.json" ],