@0x:contracts-staking Added unit tests for MixinExchangeManager

contracts/staking/contracts/test/TestExchangeManager.sol

@@ -0,0 +1,42 @@
+/*
+
+  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 TestExchangeManager is
+    Staking
+{
+    function setValidExchange(address exchange)
+        external
+    {
+        validExchanges[exchange] = true;
+    }
+
+    function onlyExchangeFunction()
+        external
+        view
+        onlyExchange
+        returns (bool)
+    {
+        return true;
+    }
+}

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

contracts/staking/src/artifacts.ts

@@ -40,6 +40,7 @@ import * as TestAssertStorageParams from '../generated-artifacts/TestAssertStora
 import * as TestCobbDouglas from '../generated-artifacts/TestCobbDouglas.json';
 import * as TestCumulativeRewardTracking from '../generated-artifacts/TestCumulativeRewardTracking.json';
 import * as TestDelegatorRewards from '../generated-artifacts/TestDelegatorRewards.json';
+import * as TestExchangeManager from '../generated-artifacts/TestExchangeManager.json';
 import * as TestFinalizer from '../generated-artifacts/TestFinalizer.json';
 import * as TestInitTarget from '../generated-artifacts/TestInitTarget.json';
 import * as TestLibFixedMath from '../generated-artifacts/TestLibFixedMath.json';
@@ -89,6 +90,7 @@ export const artifacts = {
     TestCobbDouglas: TestCobbDouglas as ContractArtifact,
     TestCumulativeRewardTracking: TestCumulativeRewardTracking as ContractArtifact,
     TestDelegatorRewards: TestDelegatorRewards as ContractArtifact,
+    TestExchangeManager: TestExchangeManager as ContractArtifact,
     TestFinalizer: TestFinalizer as ContractArtifact,
     TestInitTarget: TestInitTarget as ContractArtifact,
     TestLibFixedMath: TestLibFixedMath as ContractArtifact,

contracts/staking/src/wrappers.ts

@@ -38,6 +38,7 @@ export * from '../generated-wrappers/test_assert_storage_params';
 export * from '../generated-wrappers/test_cobb_douglas';
 export * from '../generated-wrappers/test_cumulative_reward_tracking';
 export * from '../generated-wrappers/test_delegator_rewards';
+export * from '../generated-wrappers/test_exchange_manager';
 export * from '../generated-wrappers/test_finalizer';
 export * from '../generated-wrappers/test_init_target';
 export * from '../generated-wrappers/test_lib_fixed_math';

contracts/staking/test/exchange_test.ts

@@ -1,61 +0,0 @@
-import { ERC20Wrapper } from '@0x/contracts-asset-proxy';
-import { blockchainTests, expect } from '@0x/contracts-test-utils';
-import { StakingRevertErrors } from '@0x/order-utils';
-import * as _ from 'lodash';
-
-import { deployAndConfigureContractsAsync, StakingApiWrapper } from './utils/api_wrapper';
-
-// tslint:disable:no-unnecessary-type-assertion
-blockchainTests('Exchange Integrations', env => {
-    // tokens & addresses
-    let accounts: string[];
-    let owner: string;
-    let exchange: string;
-    // wrappers
-    let stakingApiWrapper: StakingApiWrapper;
-    let erc20Wrapper: ERC20Wrapper;
-    // tests
-    before(async () => {
-        // create accounts
-        accounts = await env.getAccountAddressesAsync();
-        owner = accounts[0];
-        exchange = accounts[1];
-        // set up ERC20Wrapper
-        erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner);
-        // deploy staking contracts
-        stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper);
-    });
-    blockchainTests.resets('Exchange Tracking in Staking Contract', () => {
-        it('basic exchange tracking', async () => {
-            const { validExchanges, addExchangeAddress, removeExchangeAddress } = stakingApiWrapper.stakingContract;
-            // 1 try querying an invalid addresses
-            const invalidAddress = '0x0000000000000000000000000000000000000001';
-            const isInvalidAddressValid = await validExchanges.callAsync(invalidAddress);
-            expect(isInvalidAddressValid).to.be.false();
-            // 2 add valid address
-            await addExchangeAddress.awaitTransactionSuccessAsync(exchange);
-            const isValidAddressValid = await validExchanges.callAsync(exchange);
-            expect(isValidAddressValid).to.be.true();
-            // 3 try adding valid address again
-            let revertError = new StakingRevertErrors.ExchangeManagerError(
-                StakingRevertErrors.ExchangeManagerErrorCodes.ExchangeAlreadyRegistered,
-                exchange,
-            );
-            let tx = addExchangeAddress.awaitTransactionSuccessAsync(exchange);
-            await expect(tx).to.revertWith(revertError);
-            // 4 remove valid address
-            await removeExchangeAddress.awaitTransactionSuccessAsync(exchange);
-            const isValidAddressStillValid = await validExchanges.callAsync(exchange);
-            expect(isValidAddressStillValid).to.be.false();
-            // 5 try removing valid address again
-            revertError = new StakingRevertErrors.ExchangeManagerError(
-                StakingRevertErrors.ExchangeManagerErrorCodes.ExchangeNotRegistered,
-                exchange,
-            );
-            tx = removeExchangeAddress.awaitTransactionSuccessAsync(exchange);
-            await expect(tx).to.revertWith(revertError);
-            // @todo should not be able to add / remove an exchange if not contract owner.
-        });
-    });
-});
-// tslint:enable:no-unnecessary-type-assertion

contracts/staking/test/unit_tests/exchange_test.ts

@@ -0,0 +1,131 @@
+import { blockchainTests, expect } from '@0x/contracts-test-utils';
+import { StakingRevertErrors } from '@0x/order-utils';
+import { AuthorizableRevertErrors } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { artifacts, TestExchangeManagerContract } from '../../src';
+
+blockchainTests.resets.only('Exchange Unit Tests', env => {
+    // Addresses
+    let nonOwner: string;
+    let owner: string;
+    let nonExchange: string;
+    let exchange: string;
+    let nonAuthority: string;
+    let authority: string;
+
+    // Exchange Manager
+    let exchangeManager: TestExchangeManagerContract;
+
+    before(async () => {
+        // Set up addresses for testing.
+        [nonOwner, owner, nonExchange, exchange, nonAuthority, authority] = await env.getAccountAddressesAsync();
+
+        // Deploy the Exchange Manager contract.
+        exchangeManager = await TestExchangeManagerContract.deployFrom0xArtifactAsync(
+            artifacts.TestExchangeManager,
+            env.provider,
+            {
+                ...env.txDefaults,
+                from: owner,
+            },
+            artifacts,
+        );
+
+        // Register the exchange.
+        await exchangeManager.setValidExchange.awaitTransactionSuccessAsync(exchange);
+
+        // Register an authority.
+        await exchangeManager.addAuthorizedAddress.awaitTransactionSuccessAsync(authority, { from: owner });
+    });
+
+    describe('onlyExchange', () => {
+        it('should revert if called by an unregistered exchange', async () => {
+            const expectedError = new StakingRevertErrors.OnlyCallableByExchangeError(nonExchange);
+            return expect(exchangeManager.onlyExchangeFunction.callAsync({ from: nonExchange })).to.revertWith(
+                expectedError,
+            );
+        });
+
+        it('should succeed if called by a registered exchange', async () => {
+            const didSucceed = await exchangeManager.onlyExchangeFunction.callAsync({ from: exchange });
+            expect(didSucceed).to.be.true();
+        });
+    });
+
+    describe('addExchangeAddress', () => {
+        it('should revert if called by an unauthorized address', async () => {
+            const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(nonAuthority);
+            const tx = exchangeManager.addExchangeAddress.awaitTransactionSuccessAsync(nonExchange, {
+                from: nonAuthority,
+            });
+            return expect(tx).to.revertWith(expectedError);
+        });
+
+        it('should successfully add an exchange if called by the (authorized) owner', async () => {
+            // Register a new exchange.
+            await exchangeManager.addExchangeAddress.awaitTransactionSuccessAsync(nonExchange, { from: owner });
+
+            // Ensure that the exchange was successfully registered.
+            const isValidExchange = await exchangeManager.validExchanges.callAsync(nonExchange);
+            expect(isValidExchange).to.be.true();
+        });
+
+        it('should successfully add an exchange if called by an authorized address', async () => {
+            // Register a new exchange.
+            await exchangeManager.addExchangeAddress.awaitTransactionSuccessAsync(nonExchange, { from: authority });
+
+            // Ensure that the exchange was successfully registered.
+            const isValidExchange = await exchangeManager.validExchanges.callAsync(nonExchange);
+            expect(isValidExchange).to.be.true();
+        });
+
+        it('should fail to add an exchange redundantly', async () => {
+            const expectedError = new StakingRevertErrors.ExchangeManagerError(
+                StakingRevertErrors.ExchangeManagerErrorCodes.ExchangeAlreadyRegistered,
+                exchange,
+            );
+            const tx = exchangeManager.addExchangeAddress.awaitTransactionSuccessAsync(exchange, { from: authority });
+            return expect(tx).to.revertWith(expectedError);
+        });
+    });
+
+    describe('removeExchangeAddress', () => {
+        it('should revert if called by an unauthorized address', async () => {
+            const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(nonAuthority);
+            const tx = exchangeManager.removeExchangeAddress.awaitTransactionSuccessAsync(exchange, {
+                from: nonAuthority,
+            });
+            return expect(tx).to.revertWith(expectedError);
+        });
+
+        it('should successfully remove an exchange if called by the (authorized) owner', async () => {
+            // Remove the registered exchange.
+            await exchangeManager.removeExchangeAddress.awaitTransactionSuccessAsync(exchange, { from: owner });
+
+            // Ensure that the exchange was removed.
+            const isValidExchange = await exchangeManager.validExchanges.callAsync(exchange);
+            expect(isValidExchange).to.be.false();
+        });
+
+        it('should successfully remove a registered exchange if called by an authorized address', async () => {
+            // Remove the registered exchange.
+            await exchangeManager.removeExchangeAddress.awaitTransactionSuccessAsync(exchange, { from: authority });
+
+            // Ensure that the exchange was removed.
+            const isValidExchange = await exchangeManager.validExchanges.callAsync(exchange);
+            expect(isValidExchange).to.be.false();
+        });
+
+        it('should fail to remove an exchange redundantly', async () => {
+            const expectedError = new StakingRevertErrors.ExchangeManagerError(
+                StakingRevertErrors.ExchangeManagerErrorCodes.ExchangeNotRegistered,
+                nonExchange,
+            );
+            const tx = exchangeManager.removeExchangeAddress.awaitTransactionSuccessAsync(nonExchange, {
+                from: authority,
+            });
+            return expect(tx).to.revertWith(expectedError);
+        });
+    });
+});

contracts/staking/tsconfig.json

@@ -38,6 +38,7 @@
         "generated-artifacts/TestCobbDouglas.json",
         "generated-artifacts/TestCumulativeRewardTracking.json",
         "generated-artifacts/TestDelegatorRewards.json",
+        "generated-artifacts/TestExchangeManager.json",
         "generated-artifacts/TestFinalizer.json",
         "generated-artifacts/TestInitTarget.json",
         "generated-artifacts/TestLibFixedMath.json",

contracts/utils/contracts/src/Authorizable.sol

@@ -151,6 +151,6 @@ contract Authorizable is
         delete authorized[target];
         authorities[index] = authorities[authorities.length - 1];
         authorities.length -= 1;
-        emit AuthorizedAddressRemoved(target, msg.sender);        
+        emit AuthorizedAddressRemoved(target, msg.sender);
     }
 }