From 4415e00b388fadcf02f3a33eb00d7ac4ba60b6e6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 18 Dec 2019 13:37:17 -0800 Subject: [PATCH] Added more integration tests for DydxBridge with the Exchange (demonstrates use cases) --- .../contracts/test/TestDydxBridge.sol | 82 +++++++-- contracts/asset-proxy/test/dydx_bridge.ts | 10 +- .../test/bridges/deploy_dydx_bridge.ts | 2 + .../test/bridges/dydx_bridge_mainnet_test.ts | 12 +- .../test/exchange/fill_dydx_order_test.ts | 171 ++++++++++++++---- 5 files changed, 219 insertions(+), 58 deletions(-) diff --git a/contracts/asset-proxy/contracts/test/TestDydxBridge.sol b/contracts/asset-proxy/contracts/test/TestDydxBridge.sol index 1bf9c54d27..31dcc5359f 100644 --- a/contracts/asset-proxy/contracts/test/TestDydxBridge.sol +++ b/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; - bool private shouldRevertOnOperate; + address private _testTokenAddress; + 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`. - function balanceOf(address holder) + /// @dev Returns test token. + function getTestToken() external - view - returns (uint256) + returns (address) { - return balances[holder]; + return _testTokenAddress; } /// @dev overrides `_getDydxAddress()` from `DeploymentConstants` to return this address. diff --git a/contracts/asset-proxy/test/dydx_bridge.ts b/contracts/asset-proxy/test/dydx_bridge.ts index 1198192f6c..b9021e9e56 100644 --- a/contracts/asset-proxy/test/dydx_bridge.ts +++ b/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(); }); diff --git a/contracts/integrations/test/bridges/deploy_dydx_bridge.ts b/contracts/integrations/test/bridges/deploy_dydx_bridge.ts index de8f87f11d..f456772d51 100644 --- a/contracts/integrations/test/bridges/deploy_dydx_bridge.ts +++ b/contracts/integrations/test/bridges/deploy_dydx_bridge.ts @@ -10,11 +10,13 @@ export async function deployDydxBridgeAsync( deployment: DeploymentManager, environment: BlockchainTestsEnvironment, ): Promise { + const tokenHolders = deployment.accounts; const dydxBridge = await TestDydxBridgeContract.deployFrom0xArtifactAsync( assetProxyArtifacts.TestDydxBridge, environment.provider, deployment.txDefaults, assetProxyArtifacts, + tokenHolders, ); return dydxBridge; } diff --git a/contracts/integrations/test/bridges/dydx_bridge_mainnet_test.ts b/contracts/integrations/test/bridges/dydx_bridge_mainnet_test.ts index 33a6ba6349..2187f8b942 100644 --- a/contracts/integrations/test/bridges/dydx_bridge_mainnet_test.ts +++ b/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(expectedEvent.accountNumber, 'accountNumber').to.bignumber.equal(log.args.accountNumber); - expect(expectedEvent.market, 'market').to.bignumber.equal(log.args.market); - expect(expectedEvent.from, 'from').to.equal(log.args.from); + expect(log.args.accountOwner, 'accountOwner').to.equal(expectedEvent.accountOwner); + expect(log.args.accountNumber, 'accountNumber').to.bignumber.equal(expectedEvent.accountNumber); + expect(log.args.market, 'market').to.bignumber.equal(expectedEvent.market); + 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]); } }; diff --git a/contracts/integrations/test/exchange/fill_dydx_order_test.ts b/contracts/integrations/test/exchange/fill_dydx_order_test.ts index 874c11f3ed..2c6c82991b 100644 --- a/contracts/integrations/test/exchange/fill_dydx_order_test.ts +++ b/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 takerAssetAmount = toBaseUnitAmount(1); + const dydxConversionRateNumerator = toBaseUnitAmount(6); + const dydxConversionRateDenominator = toBaseUnitAmount(1); let maker: Maker; let taker: Taker; + let orderConfig: Partial; + let dydxBridgeProxyAssetData: string; const defaultDepositAction = { actionType: DydxBridgeActionType.Deposit as number, accountId: constants.ZERO_AMOUNT, marketId, - // The bridge is passed the `makerAssetFillAmount` and we - // want to compute the input `takerAssetFillAmount` - // => multiply by `takerAssetAmount` / `makerAssetAmount`. - conversionRateNumerator: takerAssetAmount, - conversionRateDenominator: makerAssetAmount, + conversionRateNumerator: dydxConversionRateNumerator, + conversionRateDenominator: dydxConversionRateDenominator, }; 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 - .ERC20Bridge(testContract.address, testContract.address, encodedBridgeData) + const testTokenAddress = await testContract.getTestToken().callAsync(); + dydxBridgeProxyAssetData = deployment.assetDataEncoder + .ERC20Bridge(testTokenAddress, testContract.address, encodedBridgeData) .getABIEncodedTransactionData(); [makerToken, takerToken] = deployment.tokens.erc20; - // Configure Maker & Taker. - const orderConfig = { - makerAssetAmount, - takerAssetAmount, - makerAssetData: bridgeProxyAssetData, + // Configure default order parameters. + orderConfig = { + makerAssetAmount: toBaseUnitAmount(1), + takerAssetAmount: toBaseUnitAmount(1), + 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, - takerFee: constants.ZERO_AMOUNT, + makerFee: toBaseUnitAmount(1), + 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 | LogEntry>): void => { + interface DydxFillResults { + makerAssetFilledAmount: BigNumber; + takerAssetFilledAmount: BigNumber; + makerFeePaid: BigNumber; + takerFeePaid: BigNumber; + amountDepositedIntoDydx: BigNumber; + amountWithdrawnFromDydx: BigNumber; + } + const fillOrder = async ( + signedOrder: SignedOrder, + customTakerAssetFillAmount?: BigNumber, + ): Promise => { + // 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; - 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; - 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; - const amountWithdrawnFromDydx = dydxWithdrawEvent.args.amountValue; - // Assert fill amounts match amounts deposited/withdrawn from dydx. - expect(makerAssetFilledAmount).to.bignumber.equal(amountWithdrawnFromDydx); - expect(takerAssetFilledAmount).to.bignumber.equal(amountDepositedIntoDydx); + // Return values of interest for assertions. + return { + 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 () => { - const signedOrder = await maker.signOrderAsync(); - const tx = await taker.fillOrderAsync(signedOrder, signedOrder.takerAssetAmount); - verifyEvents(tx.logs); + it('should successfully 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); + 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 () => { - const signedOrder = await maker.signOrderAsync(); - const tx = await taker.fillOrderAsync(signedOrder, signedOrder.takerAssetAmount.div(2)); - verifyEvents(tx.logs); + it('should successfully fill a dydx order (DydxBridge used in takerAssetData)', async () => { + const signedOrder = await maker.signOrderAsync({ + // Match the dydx conversion rate when using the bridge in the takerAssetData. + 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); }); }); });