Added more integration tests for DydxBridge with the Exchange (demonstrates use cases)

contracts/asset-proxy/contracts/test/TestDydxBridge.sol

@@ -19,9 +19,49 @@
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
+import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol";
 import "../src/bridges/DydxBridge.sol";
 
 
+contract TestDydxBridgeToken {
+
+    uint256 private constant INIT_HOLDER_BALANCE = 10 * 10**18; // 10 tokens
+    mapping (address => uint256) private _balances;
+
+    /// @dev Sets initial balance of token holders.
+    constructor(address[] memory holders)
+        public
+    {
+        for (uint256 i = 0; i != holders.length; ++i) {
+            _balances[holders[i]] = INIT_HOLDER_BALANCE;
+        }
+        _balances[msg.sender] = INIT_HOLDER_BALANCE;
+    }
+
+    /// @dev Basic transferFrom implementation.
+    function transferFrom(address from, address to, uint256 amount)
+        external
+        returns (bool)
+    {
+        if (_balances[from] < amount || _balances[to] + amount < _balances[to]) {
+            return false;
+        }
+        _balances[from] -= amount;
+        _balances[to] += amount;
+        return true;
+    }
+
+    /// @dev Returns balance of `holder`.
+    function balanceOf(address holder)
+        external
+        view
+        returns (uint256)
+    {
+        return _balances[holder];
+    }
+}
+
+
 // solhint-disable space-after-comma
 contract TestDydxBridge is
     IDydx,
@@ -29,8 +69,8 @@ contract TestDydxBridge is
 {
 
     address private constant ALWAYS_REVERT_ADDRESS = address(1);
-    mapping (address => uint256) private balances;
+    address private _testTokenAddress;
-    bool private shouldRevertOnOperate;
+    bool private _shouldRevertOnOperate;
 
     event OperateAccount(
         address owner,
@@ -51,6 +91,13 @@ contract TestDydxBridge is
         bytes data
     );
 
+    constructor(address[] memory holders)
+        public
+    {
+        // Deploy a test token. This represents the asset being deposited/withdrawn from dydx.
+        _testTokenAddress = address(new TestDydxBridgeToken(holders));
+    }
+
     /// @dev Simulates `operate` in dydx contract.
     ///      Emits events so that arguments can be validated client-side.
     function operate(
@@ -59,7 +106,7 @@ contract TestDydxBridge is
     )
         external
     {
-        if (shouldRevertOnOperate) {
+        if (_shouldRevertOnOperate) {
             revert("TestDydxBridge/SHOULD_REVERT_ON_OPERATE");
         }
 
@@ -86,9 +133,23 @@ contract TestDydxBridge is
             );
 
             if (actions[i].actionType == IDydx.ActionType.Withdraw) {
-                balances[actions[i].otherAddress] += actions[i].amount.value;
+                require(
+                    IERC20Token(_testTokenAddress).transferFrom(
+                        address(this),
+                        actions[i].otherAddress,
+                        actions[i].amount.value
+                    ),
+                    "TestDydxBridge/WITHDRAW_FAILED"
+                );
             } else if (actions[i].actionType == IDydx.ActionType.Deposit) {
-                balances[actions[i].otherAddress] -= actions[i].amount.value;
+                require(
+                    IERC20Token(_testTokenAddress).transferFrom(
+                        actions[i].otherAddress,
+                        address(this),
+                        actions[i].amount.value
+                    ),
+                    "TestDydxBridge/DEPOSIT_FAILED"
+                );
             } else {
                 revert("TestDydxBridge/UNSUPPORTED_ACTION");
             }
@@ -99,16 +160,15 @@ contract TestDydxBridge is
     function setRevertOnOperate(bool shouldRevert)
         external
     {
-        shouldRevertOnOperate = shouldRevert;
+        _shouldRevertOnOperate = shouldRevert;
     }
 
-    /// @dev Returns balance of `holder`.
+    /// @dev Returns test token.
-    function balanceOf(address holder)
+    function getTestToken()
         external
-        view
+        returns (address)
-        returns (uint256)
     {
-        return balances[holder];
+        return _testTokenAddress;
     }
 
     /// @dev overrides `_getDydxAddress()` from `DeploymentConstants` to return this address.

contracts/asset-proxy/test/dydx_bridge.ts

@@ -12,18 +12,17 @@ import { TestDydxBridgeContract, TestDydxBridgeEvents } from './wrappers';
 blockchainTests.resets('DydxBridge unit tests', env => {
     const defaultAccountNumber = new BigNumber(1);
     const marketId = new BigNumber(2);
-    let tokenAddress: string;
     const defaultAmount = new BigNumber(4);
     const notAuthorized = '0x0000000000000000000000000000000000000001';
     const defaultDepositAction = {
-        actionType: DydxBridgeActionType.Deposit as number,
+        actionType: DydxBridgeActionType.Deposit,
         accountId: constants.ZERO_AMOUNT,
         marketId,
         conversionRateNumerator: constants.ZERO_AMOUNT,
         conversionRateDenominator: constants.ZERO_AMOUNT,
     };
     const defaultWithdrawAction = {
-        actionType: DydxBridgeActionType.Withdraw as number,
+        actionType: DydxBridgeActionType.Withdraw,
         accountId: constants.ZERO_AMOUNT,
         marketId,
         conversionRateNumerator: constants.ZERO_AMOUNT,
@@ -48,6 +47,7 @@ blockchainTests.resets('DydxBridge unit tests', env => {
             env.provider,
             env.txDefaults,
             artifacts,
+            [accountOwner, receiver],
         );
 
         // Deploy test erc20 bridge proxy
@@ -61,7 +61,6 @@ blockchainTests.resets('DydxBridge unit tests', env => {
 
         // Setup asset data encoder
         assetDataEncoder = new IAssetDataContract(constants.NULL_ADDRESS, env.provider);
-        tokenAddress = testContract.address;
     });
 
     describe('bridgeTransferFrom()', () => {
@@ -289,8 +288,9 @@ blockchainTests.resets('DydxBridge unit tests', env => {
         let assetData: string;
 
         before(async () => {
+            const testTokenAddress = await testContract.getTestToken().callAsync();
             assetData = assetDataEncoder
-                .ERC20Bridge(tokenAddress, testContract.address, dydxBridgeDataEncoder.encode({ bridgeData }))
+                .ERC20Bridge(testTokenAddress, testContract.address, dydxBridgeDataEncoder.encode({ bridgeData }))
                 .getABIEncodedTransactionData();
         });
 

contracts/integrations/test/bridges/deploy_dydx_bridge.ts

@@ -10,11 +10,13 @@ export async function deployDydxBridgeAsync(
     deployment: DeploymentManager,
     environment: BlockchainTestsEnvironment,
 ): Promise<TestDydxBridgeContract> {
+    const tokenHolders = deployment.accounts;
     const dydxBridge = await TestDydxBridgeContract.deployFrom0xArtifactAsync(
         assetProxyArtifacts.TestDydxBridge,
         environment.provider,
         deployment.txDefaults,
         assetProxyArtifacts,
+        tokenHolders,
     );
     return dydxBridge;
 }

contracts/integrations/test/bridges/dydx_bridge_mainnet_test.ts

@@ -106,16 +106,16 @@ blockchainTests.resets.fork('Mainnet dydx bridge tests', env => {
                     log.event === 'LogDeposit'
                         ? expectedDepositEvents[nextExpectedDepositEventIdx++]
                         : expectedWithdrawEvents[nextExpectedWithdrawEventIdx++];
-                expect(expectedEvent.accountOwner, 'accountOwner').to.equal(log.args.accountOwner);
+                expect(log.args.accountOwner, 'accountOwner').to.equal(expectedEvent.accountOwner);
-                expect(expectedEvent.accountNumber, 'accountNumber').to.bignumber.equal(log.args.accountNumber);
+                expect(log.args.accountNumber, 'accountNumber').to.bignumber.equal(expectedEvent.accountNumber);
-                expect(expectedEvent.market, 'market').to.bignumber.equal(log.args.market);
+                expect(log.args.market, 'market').to.bignumber.equal(expectedEvent.market);
-                expect(expectedEvent.from, 'from').to.equal(log.args.from);
+                expect(log.args.from, 'from').to.equal(expectedEvent.from);
                 // We only check the first update field because it's the delta balance (amount deposited).
                 // The next field is the new total, which depends on interest rates at the time of execution.
-                expect(expectedEvent.update[0][0], 'update sign').to.equal(log.args.update[0][0]);
+                expect(log.args.update[0][0], 'update sign').to.equal(expectedEvent.update[0][0]);
                 const updateValueHex = log.args.update[0][1]._hex;
                 const updateValueBn = new BigNumber(updateValueHex, 16);
-                expect(expectedEvent.update[0][1], 'update value').to.bignumber.equal(updateValueBn);
+                expect(updateValueBn, 'update value').to.bignumber.equal(expectedEvent.update[0][1]);
             }
         };
 

contracts/integrations/test/exchange/fill_dydx_order_test.ts

@@ -1,8 +1,9 @@
 import { DydxBridgeActionType, dydxBridgeDataEncoder, TestDydxBridgeContract } from '@0x/contracts-asset-proxy';
 import { DummyERC20TokenContract } from '@0x/contracts-erc20';
 import { blockchainTests, constants, describe, expect, toBaseUnitAmount } from '@0x/contracts-test-utils';
+import { SignedOrder } from '@0x/order-utils';
 import { BigNumber } from '@0x/utils';
-import { DecodedLogArgs, LogEntry, LogWithDecodedArgs } from 'ethereum-types';
+import { DecodedLogArgs, LogWithDecodedArgs } from 'ethereum-types';
 import * as _ from 'lodash';
 
 import { deployDydxBridgeAsync } from '../bridges/deploy_dydx_bridge';
@@ -16,19 +17,18 @@ blockchainTests.resets('Exchange fills dydx orders', env => {
     let makerToken: DummyERC20TokenContract;
     let takerToken: DummyERC20TokenContract;
     const marketId = new BigNumber(3);
-    const makerAssetAmount = toBaseUnitAmount(6);
+    const dydxConversionRateNumerator = toBaseUnitAmount(6);
-    const takerAssetAmount = toBaseUnitAmount(1);
+    const dydxConversionRateDenominator = toBaseUnitAmount(1);
     let maker: Maker;
     let taker: Taker;
+    let orderConfig: Partial<SignedOrder>;
+    let dydxBridgeProxyAssetData: string;
     const defaultDepositAction = {
         actionType: DydxBridgeActionType.Deposit as number,
         accountId: constants.ZERO_AMOUNT,
         marketId,
-        // The bridge is passed the `makerAssetFillAmount` and we
+        conversionRateNumerator: dydxConversionRateNumerator,
-        // want to compute the input `takerAssetFillAmount`
+        conversionRateDenominator: dydxConversionRateDenominator,
-        //   => multiply by `takerAssetAmount` / `makerAssetAmount`.
-        conversionRateNumerator: takerAssetAmount,
-        conversionRateDenominator: makerAssetAmount,
     };
     const defaultWithdrawAction = {
         actionType: DydxBridgeActionType.Withdraw as number,
@@ -49,16 +49,17 @@ blockchainTests.resets('Exchange fills dydx orders', env => {
         });
         testContract = await deployDydxBridgeAsync(deployment, env);
         const encodedBridgeData = dydxBridgeDataEncoder.encode({ bridgeData });
-        const bridgeProxyAssetData = deployment.assetDataEncoder
+        const testTokenAddress = await testContract.getTestToken().callAsync();
-            .ERC20Bridge(testContract.address, testContract.address, encodedBridgeData)
+        dydxBridgeProxyAssetData = deployment.assetDataEncoder
+            .ERC20Bridge(testTokenAddress, testContract.address, encodedBridgeData)
             .getABIEncodedTransactionData();
         [makerToken, takerToken] = deployment.tokens.erc20;
 
-        // Configure Maker & Taker.
+        // Configure default order parameters.
-        const orderConfig = {
+        orderConfig = {
-            makerAssetAmount,
+            makerAssetAmount: toBaseUnitAmount(1),
-            takerAssetAmount,
+            takerAssetAmount: toBaseUnitAmount(1),
-            makerAssetData: bridgeProxyAssetData,
+            makerAssetData: deployment.assetDataEncoder.ERC20Token(makerToken.address).getABIEncodedTransactionData(),
             takerAssetData: deployment.assetDataEncoder.ERC20Token(takerToken.address).getABIEncodedTransactionData(),
             // Not important for this test.
             feeRecipientAddress: constants.NULL_ADDRESS,
@@ -68,14 +69,19 @@ blockchainTests.resets('Exchange fills dydx orders', env => {
             takerFeeAssetData: deployment.assetDataEncoder
                 .ERC20Token(takerToken.address)
                 .getABIEncodedTransactionData(),
-            makerFee: constants.ZERO_AMOUNT,
+            makerFee: toBaseUnitAmount(1),
-            takerFee: constants.ZERO_AMOUNT,
+            takerFee: toBaseUnitAmount(1),
         };
+
+        // Configure maker.
         maker = new Maker({
             name: 'Maker',
             deployment,
             orderConfig,
         });
+        await maker.configureERC20TokenAsync(makerToken, deployment.assetProxies.erc20Proxy.address);
+
+        // Configure taker.
         taker = new Taker({
             name: 'Taker',
             deployment,
@@ -88,46 +94,139 @@ blockchainTests.resets('Exchange fills dydx orders', env => {
     });
 
     describe('fillOrder', () => {
-        const verifyEvents = (logs: Array<LogWithDecodedArgs<DecodedLogArgs> | LogEntry>): void => {
+        interface DydxFillResults {
+            makerAssetFilledAmount: BigNumber;
+            takerAssetFilledAmount: BigNumber;
+            makerFeePaid: BigNumber;
+            takerFeePaid: BigNumber;
+            amountDepositedIntoDydx: BigNumber;
+            amountWithdrawnFromDydx: BigNumber;
+        }
+        const fillOrder = async (
+            signedOrder: SignedOrder,
+            customTakerAssetFillAmount?: BigNumber,
+        ): Promise<DydxFillResults> => {
+            // Fill order
+            const takerAssetFillAmount =
+                customTakerAssetFillAmount !== undefined ? customTakerAssetFillAmount : signedOrder.takerAssetAmount;
+            const tx = await taker.fillOrderAsync(signedOrder, takerAssetFillAmount);
+
             // Extract values from fill event.
             // tslint:disable no-unnecessary-type-assertion
-            const fillEvent = _.find(logs, log => {
+            const fillEvent = _.find(tx.logs, log => {
                 return (log as any).event === 'Fill';
             }) as LogWithDecodedArgs<DecodedLogArgs>;
-            const makerAssetFilledAmount = fillEvent.args.makerAssetFilledAmount;
-            const takerAssetFilledAmount = fillEvent.args.takerAssetFilledAmount;
 
             // Extract amount deposited into dydx from maker.
-            const dydxDepositEvent = _.find(logs, log => {
+            const dydxDepositEvent = _.find(tx.logs, log => {
                 return (
                     (log as any).event === 'OperateAction' &&
                     (log as any).args.actionType === DydxBridgeActionType.Deposit
                 );
             }) as LogWithDecodedArgs<DecodedLogArgs>;
-            const amountDepositedIntoDydx = dydxDepositEvent.args.amountValue;
 
             // Extract amount withdrawn from dydx to taker.
-            const dydxWithdrawEvent = _.find(logs, log => {
+            const dydxWithdrawEvent = _.find(tx.logs, log => {
                 return (
                     (log as any).event === 'OperateAction' &&
                     (log as any).args.actionType === DydxBridgeActionType.Withdraw
                 );
             }) as LogWithDecodedArgs<DecodedLogArgs>;
-            const amountWithdrawnFromDydx = dydxWithdrawEvent.args.amountValue;
 
-            // Assert fill amounts match amounts deposited/withdrawn from dydx.
+            // Return values of interest for assertions.
-            expect(makerAssetFilledAmount).to.bignumber.equal(amountWithdrawnFromDydx);
+            return {
-            expect(takerAssetFilledAmount).to.bignumber.equal(amountDepositedIntoDydx);
+                makerAssetFilledAmount: fillEvent.args.makerAssetFilledAmount,
+                takerAssetFilledAmount: fillEvent.args.takerAssetFilledAmount,
+                makerFeePaid: fillEvent.args.makerFeePaid,
+                takerFeePaid: fillEvent.args.takerFeePaid,
+                amountDepositedIntoDydx: dydxDepositEvent.args.amountValue,
+                amountWithdrawnFromDydx: dydxWithdrawEvent.args.amountValue,
+            };
         };
-        it('should successfully fill a dydx order', async () => {
+        it('should successfully fill a dydx order (DydxBridge used in makerAssetData)', async () => {
-            const signedOrder = await maker.signOrderAsync();
+            const signedOrder = await maker.signOrderAsync({
-            const tx = await taker.fillOrderAsync(signedOrder, signedOrder.takerAssetAmount);
+                // Invert the dydx conversion rate when using the bridge in the makerAssetData.
-            verifyEvents(tx.logs);
+                makerAssetData: dydxBridgeProxyAssetData,
+                makerAssetAmount: dydxConversionRateDenominator,
+                takerAssetAmount: dydxConversionRateNumerator,
+            });
+            const dydxFillResults = await fillOrder(signedOrder);
+            expect(
+                dydxFillResults.makerAssetFilledAmount,
+                'makerAssetFilledAmount should equal amountWithdrawnFromDydx',
+            ).to.bignumber.equal(dydxFillResults.amountWithdrawnFromDydx);
+            expect(
+                dydxFillResults.takerAssetFilledAmount,
+                'takerAssetFilledAmount should equal amountDepositedIntoDydx',
+            ).to.bignumber.equal(dydxFillResults.amountDepositedIntoDydx);
         });
-        it('should partially fill a dydx order', async () => {
+        it('should successfully fill a dydx order (DydxBridge used in takerAssetData)', async () => {
-            const signedOrder = await maker.signOrderAsync();
+            const signedOrder = await maker.signOrderAsync({
-            const tx = await taker.fillOrderAsync(signedOrder, signedOrder.takerAssetAmount.div(2));
+                // Match the dydx conversion rate when using the bridge in the takerAssetData.
-            verifyEvents(tx.logs);
+                takerAssetData: dydxBridgeProxyAssetData,
+                makerAssetAmount: dydxConversionRateNumerator,
+                takerAssetAmount: dydxConversionRateDenominator,
+            });
+            const dydxFillResults = await fillOrder(signedOrder);
+            expect(
+                dydxFillResults.makerAssetFilledAmount,
+                'makerAssetFilledAmount should equal amountDepositedIntoDydx',
+            ).to.bignumber.equal(dydxFillResults.amountDepositedIntoDydx);
+            expect(
+                dydxFillResults.takerAssetFilledAmount,
+                'takerAssetFilledAmount should equal amountWithdrawnFromDydx',
+            ).to.bignumber.equal(dydxFillResults.amountWithdrawnFromDydx);
+        });
+        it('should successfully fill a dydx order (DydxBridge used in makerFeeAssetData)', async () => {
+            const signedOrder = await maker.signOrderAsync({
+                // Invert the dydx conversion rate when using the bridge in the makerFeeAssetData.
+                makerFeeAssetData: dydxBridgeProxyAssetData,
+                makerFee: dydxConversionRateDenominator,
+                takerFee: dydxConversionRateNumerator,
+            });
+            const dydxFillResults = await fillOrder(signedOrder);
+            expect(
+                dydxFillResults.makerFeePaid,
+                'makerFeePaid should equal amountWithdrawnFromDydx',
+            ).to.bignumber.equal(dydxFillResults.amountWithdrawnFromDydx);
+            expect(
+                dydxFillResults.takerFeePaid,
+                'takerFeePaid should equal amountDepositedIntoDydx',
+            ).to.bignumber.equal(dydxFillResults.amountDepositedIntoDydx);
+        });
+        it('should successfully fill a dydx order (DydxBridge used in takerFeeAssetData)', async () => {
+            const signedOrder = await maker.signOrderAsync({
+                // Match the dydx conversion rate when using the bridge in the takerFeeAssetData.
+                takerFeeAssetData: dydxBridgeProxyAssetData,
+                makerFee: dydxConversionRateNumerator,
+                takerFee: dydxConversionRateDenominator,
+            });
+            const dydxFillResults = await fillOrder(signedOrder);
+            expect(
+                dydxFillResults.makerFeePaid,
+                'makerFeePaid should equal amountDepositedIntoDydx',
+            ).to.bignumber.equal(dydxFillResults.amountDepositedIntoDydx);
+            expect(
+                dydxFillResults.takerFeePaid,
+                'takerFeePaid should equal amountWithdrawnFromDydx',
+            ).to.bignumber.equal(dydxFillResults.amountWithdrawnFromDydx);
+        });
+        it('should partially fill a dydx order (DydxBridge used in makerAssetData)', async () => {
+            const signedOrder = await maker.signOrderAsync({
+                // Invert the dydx conversion rate when using the bridge in the makerAssetData.
+                makerAssetData: dydxBridgeProxyAssetData,
+                makerAssetAmount: dydxConversionRateDenominator,
+                takerAssetAmount: dydxConversionRateNumerator,
+            });
+            const dydxFillResults = await fillOrder(signedOrder, signedOrder.takerAssetAmount.div(2));
+            expect(
+                dydxFillResults.makerAssetFilledAmount,
+                'makerAssetFilledAmount should equal amountWithdrawnFromDydx',
+            ).to.bignumber.equal(dydxFillResults.amountWithdrawnFromDydx);
+            expect(
+                dydxFillResults.takerAssetFilledAmount,
+                'takerAssetFilledAmount should equal amountDepositedIntoDydx',
+            ).to.bignumber.equal(dydxFillResults.amountDepositedIntoDydx);
         });
     });
 });