Staking Proxy unit tests + Staking Proxy fallback reverts if no staking contract is attached

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))

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);
+    }
+}

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;
+    }
+}

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,
 };

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';

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

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"
     ],