Readability improvements to Staking Proxy unit tests + one extra case

contracts/staking/contracts/src/StakingProxy.sol

@@ -112,7 +112,7 @@ contract StakingProxy is
         address staking = stakingContract;
 
         // Ensure that a staking contract has been attached to the proxy.
-        if (staking == address(0)) {
+        if (staking == NIL_ADDRESS) {
             LibRichErrors.rrevert(
                 LibStakingRichErrors.ProxyDestinationCannotBeNilError()
             );

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/@(IStaking|IStakingEvents|IStakingProxy|IStorage|IStorageInit|IStructs|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewards|MixinStorage|Staking|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestExchangeManager|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibSafeDowncast|TestMixinParams|TestMixinStake|TestMixinStakeBalances|TestMixinStakeStorage|TestMixinStakingPool|TestProtocolFees|TestStaking|TestStakingNoWETH|TestStakingProxy|TestStorageLayoutAndConstants|ZrxVault).json"
+        "abis": "./generated-artifacts/@(IStaking|IStakingEvents|IStakingProxy|IStorage|IStorageInit|IStructs|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewards|MixinStorage|Staking|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestExchangeManager|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibSafeDowncast|TestMixinParams|TestMixinStake|TestMixinStakeBalances|TestMixinStakeStorage|TestMixinStakingPool|TestProtocolFees|TestProxyDestination|TestStaking|TestStakingNoWETH|TestStakingProxy|TestStakingProxyUnit|TestStorageLayoutAndConstants|ZrxVault).json"
     },
     "repository": {
         "type": "git",

contracts/staking/test/unit_tests/staking_proxy_test.ts

@@ -1,13 +1,13 @@
-import { blockchainTests, constants, expect } from '@0x/contracts-test-utils';
+import { blockchainTests, constants, expect, verifyEventsFromLogs } from '@0x/contracts-test-utils';
 import { StakingRevertErrors } from '@0x/order-utils';
 import { AuthorizableRevertErrors, BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
 import {
     artifacts,
-    StakingProxyStakingContractAttachedToProxyEventArgs,
+    StakingProxyEvents,
     TestProxyDestinationContract,
-    TestProxyDestinationInitCalledEventArgs,
+    TestProxyDestinationEvents,
     TestStakingProxyUnitContract,
 } from '../../src';
 
@@ -90,15 +90,26 @@ blockchainTests.resets('StakingProxy unit tests', env => {
             );
 
             // 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);
+            verifyEventsFromLogs(
+                txReceipt.logs,
+                [
+                    {
+                        newStakingContractAddress: testContract2.address,
+                    },
+                ],
+                StakingProxyEvents.StakingContractAttachedToProxy,
+            );
 
             // 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();
+            verifyEventsFromLogs(
+                txReceipt.logs,
+                [
+                    {
+                        initCalled: true,
+                    },
+                ],
+                TestProxyDestinationEvents.InitCalled,
+            );
 
             // Validate new staking contract address
             const finalStakingContractAddress = await testProxyContract.stakingContract.callAsync();
@@ -133,7 +144,7 @@ blockchainTests.resets('StakingProxy unit tests', env => {
             });
 
             // Validate that event was emitted
-            expect(txReceipt.logs.length).to.eq(1);
+            verifyEventsFromLogs(txReceipt.logs, [{}], StakingProxyEvents.StakingContractDetachedFromProxy);
 
             // Validate staking contract address was unset
             const finalStakingContractAddress = await testProxyContract.stakingContract.callAsync();
@@ -186,14 +197,23 @@ blockchainTests.resets('StakingProxy unit tests', env => {
                 testContract.doMath.getABIEncodedTransactionData(new BigNumber(2), new BigNumber(1)),
             ];
             const tx = testProxyContract.batchExecute.callAsync(calls);
-            const expectedError = 'Goodbye, World!';
+            const expectedError = testRevertString;
+            return expect(tx).to.revertWith(expectedError);
+        });
+
+        it('should revert if no staking contract is attached', async () => {
+            await testProxyContract.detachStakingContract.awaitTransactionSuccessAsync({ from: authorizedAddress });
+            const calls = [testContract.echo.getABIEncodedTransactionData(testString)];
+
+            const tx = testProxyContract.batchExecute.callAsync(calls);
+            const expectedError = new StakingRevertErrors.ProxyDestinationCannotBeNilError();
             return expect(tx).to.revertWith(expectedError);
         });
     });
 
     describe('assertValidStorageParams', () => {
         const validStorageParams = {
-            epochDurationInSeconds: new BigNumber(5 * 24 * 60 * 60), // 5 days
+            epochDurationInSeconds: new BigNumber(stakingConstants.ONE_DAY_IN_SECONDS * 5),
             cobbDouglasAlphaNumerator: new BigNumber(1),
             cobbDouglasAlphaDenominator: new BigNumber(1),
             rewardDelegatedStakeWeight: constants.PPM_DENOMINATOR,
@@ -218,7 +238,7 @@ blockchainTests.resets('StakingProxy unit tests', env => {
         it('should revert if `epochDurationInSeconds` is greater than 30 days', async () => {
             const invalidStorageParams = {
                 ...validStorageParams,
-                epochDurationInSeconds: new BigNumber(31 * 24 * 60 * 60), // 31 days
+                epochDurationInSeconds: new BigNumber(stakingConstants.ONE_DAY_IN_SECONDS * 31),
             };
             await testProxyContract.setTestStorageParams.awaitTransactionSuccessAsync(invalidStorageParams);
             const tx = testProxyContract.assertValidStorageParams.callAsync();

contracts/staking/test/utils/constants.ts

@@ -18,4 +18,5 @@ export const constants = {
         cobbDouglasAlphaDenominator: new BigNumber(3),
     },
     PPM,
+    ONE_DAY_IN_SECONDS: 24 * 60 * 60,
 };