diff --git a/contracts/asset-proxy/contracts/src/bridges/UniswapV2Bridge.sol b/contracts/asset-proxy/contracts/src/bridges/UniswapV2Bridge.sol index 4e3633be88..bd9e5ae9c4 100644 --- a/contracts/asset-proxy/contracts/src/bridges/UniswapV2Bridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/UniswapV2Bridge.sol @@ -78,18 +78,19 @@ contract UniswapV2Bridge is // Get our balance of `fromTokenAddress` token. state.fromTokenBalance = IERC20Token(state.fromTokenAddress).balanceOf(address(this)); - // // Grant the Uniswap router an allowance. // FIXME: REVERTING - // LibERC20Token.approve( - // state.fromTokenAddress, - // _getUniswapV2Router01Address(), - // // state.fromTokenBalance - // uint256(-1) - // ); + require(state.fromTokenBalance > 0, 'must have balance'); + + // Grant the Uniswap router an allowance. + LibERC20Token.approve( + state.fromTokenAddress, + _getUniswapV2Router01Address(), + state.fromTokenBalance + ); // Convert directly from fromTokenAddress to toTokenAddress address[] memory path = new address[](2); - path = path.append(state.fromTokenAddress); - path = path.append(toTokenAddress); + path[0] = state.fromTokenAddress; + path[1] = toTokenAddress; // Buy as much `toTokenAddress` token with `fromTokenAddress` token // and transfer it to `to`. @@ -107,7 +108,7 @@ contract UniswapV2Bridge is block.timestamp ); - state.boughtAmount = amounts[1]; + state.boughtAmount = amounts[0]; emit ERC20BridgeTransfer( // input token diff --git a/contracts/asset-proxy/contracts/test/TestUniswapV2Bridge.sol b/contracts/asset-proxy/contracts/test/TestUniswapV2Bridge.sol index 0b30418761..53b63ac4ca 100644 --- a/contracts/asset-proxy/contracts/test/TestUniswapV2Bridge.sol +++ b/contracts/asset-proxy/contracts/test/TestUniswapV2Bridge.sol @@ -25,15 +25,8 @@ import "@0x/contracts-utils/contracts/src/LibAddressArray.sol"; import "../src/bridges/UniswapV2Bridge.sol"; import "../src/interfaces/IUniswapV2Router01.sol"; - -/// @dev A minimalist ERC20/WETH token. -contract TestToken { - - using LibSafeMath for uint256; - - mapping (address => uint256) public balances; - string private _nextRevertReason; - +contract TestEventsRaiser { + event TokenTransfer( address token, address from, @@ -46,20 +39,65 @@ contract TestToken { uint256 allowance ); - /// @dev Set the balance for `owner`. - function setBalance(address owner) + event SwapExactTokensForTokens( + uint amountIn, + uint amountOutMin, + address toTokenAddress, + address to, + uint deadline + ); + + function raiseTokenTransfer( + address from, + address to, + uint256 amount + ) external - payable { - balances[owner] = msg.value; + emit TokenTransfer( + msg.sender, + from, + to, + amount + ); } - /// @dev Set the revert reason for `transfer()`, - /// `deposit()`, and `withdraw()`. - function setRevertReason(string calldata reason) + function raiseTokenApprove(address spender, uint256 allowance) external { + emit TokenApprove(spender, allowance); + } + + function raiseSwapExactTokensForTokens( + uint amountIn, + uint amountOutMin, + address toTokenAddress, + address to, + uint deadline + ) external + { + emit SwapExactTokensForTokens( + amountIn, + amountOutMin, + toTokenAddress, + to, + deadline + ); + } +} + +/// @dev A minimalist ERC20 token. +contract TestToken { + + using LibSafeMath for uint256; + + mapping (address => uint256) public balances; + string private _nextRevertReason; + + /// @dev Set the balance for `owner`. + function setBalance(address owner, uint256 balance) external + payable { - _nextRevertReason = reason; + balances[owner] = balance; } /// @dev Just emits a TokenTransfer event on the caller @@ -67,8 +105,7 @@ contract TestToken { external returns (bool) { - _revertIfReasonExists(); - emit TokenTransfer(msg.sender, msg.sender, to, amount); + TestEventsRaiser(msg.sender).raiseTokenTransfer(msg.sender, to, amount); return true; } @@ -77,32 +114,10 @@ contract TestToken { external returns (bool) { - emit TokenApprove(spender, allowance); + TestEventsRaiser(msg.sender).raiseTokenApprove(spender, allowance); return true; } - /// @dev `IWETH.deposit()` that increases balances and calls - /// `raiseWethDeposit()` on the caller. - function deposit() - external - payable - { - _revertIfReasonExists(); - balances[msg.sender] += balances[msg.sender].safeAdd(msg.value); - // TestEventsRaiser(msg.sender).raiseWethDeposit(msg.value); - } - - /// @dev `IWETH.withdraw()` that just reduces balances and calls - /// `raiseWethWithdraw()` on the caller. - function withdraw(uint256 amount) - external - { - _revertIfReasonExists(); - balances[msg.sender] = balances[msg.sender].safeSub(amount); - msg.sender.transfer(amount); - // TestEventsRaiser(msg.sender).raiseWethWithdraw(amount); - } - function allowance(address, address) external view returns (uint256) { return 0; } @@ -115,30 +130,20 @@ contract TestToken { { return balances[owner]; } - - function _revertIfReasonExists() - private - view - { - if (bytes(_nextRevertReason).length != 0) { - revert(_nextRevertReason); - } - } } contract TestRouter is IUniswapV2Router01 { + string private _nextRevertReason; - event TokenToTokenTransferInput( - address exchange, - uint256 tokensSold, - uint256 minTokensBought, - uint256 deadline, - address recipient, - address toTokenAddress - ); + /// @dev Set the revert reason for `swapExactTokensForTokens`. + function setRevertReason(string calldata reason) + external + { + _nextRevertReason = reason; + } function swapExactTokensForTokens( uint amountIn, @@ -148,32 +153,41 @@ contract TestRouter is uint deadline ) external returns (uint[] memory amounts) { - amounts = new uint[](2); - amounts[0] = amountIn; - // amounts[1] = address(this).balance; - amounts[1] = amountOutMin; + _revertIfReasonExists(); - emit TokenToTokenTransferInput( - msg.sender, + amounts = new uint[](1); + amounts[0] = amountOutMin; + + TestEventsRaiser(msg.sender).raiseSwapExactTokensForTokens( // tokens sold amountIn, // tokens bought amountOutMin, - // deadline - deadline, + // output token (toTokenAddress) + path[1], // recipient to, - // output token (toTokenAddress) - path[1] + // deadline + deadline ); } + function _revertIfReasonExists() + private + view + { + if (bytes(_nextRevertReason).length != 0) { + revert(_nextRevertReason); + } + } + } /// @dev UniswapV2Bridge overridden to mock tokens and Uniswap router contract TestUniswapV2Bridge is - UniswapV2Bridge + UniswapV2Bridge, + TestEventsRaiser { // Token address to TestToken instance. @@ -185,22 +199,27 @@ contract TestUniswapV2Bridge is _testRouter = new TestRouter(); } - /// @dev Sets the balance of this contract for an existing token. - /// The wei attached will be the balance. - function setTokenBalance(address tokenAddress) + function getRouterAddress() external - payable + view + returns (address) { - TestToken token = _testTokens[tokenAddress]; - token.deposit.value(msg.value)(); + return address(_testRouter); } - /// @dev Sets the revert reason for an existing token. - function setTokenRevertReason(address tokenAddress, string calldata revertReason) + function setRouterRevertReason(string calldata revertReason) + external + { + _testRouter.setRevertReason(revertReason); + } + + /// @dev Sets the balance of this contract for an existing token. + /// The wei attached will be the balance. + function setTokenBalance(address tokenAddress, uint256 balance) external { TestToken token = _testTokens[tokenAddress]; - token.setRevertReason(revertReason); + token.setBalance(address(this), balance); } /// @dev Create a new token @@ -209,7 +228,6 @@ contract TestUniswapV2Bridge is address tokenAddress ) external - payable returns (TestToken token) { token = TestToken(tokenAddress); diff --git a/contracts/asset-proxy/test/uniswapv2_bridge.ts b/contracts/asset-proxy/test/uniswapv2_bridge.ts index 14fbd5ea1f..3a12126fea 100644 --- a/contracts/asset-proxy/test/uniswapv2_bridge.ts +++ b/contracts/asset-proxy/test/uniswapv2_bridge.ts @@ -4,7 +4,6 @@ import { expect, filterLogsToArguments, getRandomInteger, - getRandomPortion, randomAddress, } from '@0x/contracts-test-utils'; import { AssetProxyId } from '@0x/types'; @@ -16,11 +15,16 @@ import { artifacts } from './artifacts'; import { TestUniswapV2BridgeContract, - UniswapV2BridgeERC20BridgeTransferEventArgs, - UniswapV2BridgeEvents, + TestUniswapV2BridgeEvents as ContractEvents, + TestUniswapV2BridgeSwapExactTokensForTokensEventArgs as SwapExactTokensForTokensArgs, + // TestUniswapV2BridgeTokenToEthSwapInputEventArgs as TokenToEthSwapInputArgs, + TestUniswapV2BridgeTokenApproveEventArgs as TokenApproveArgs, + TestUniswapV2BridgeTokenTransferEventArgs as TokenTransferArgs, + // TestUniswapV2BridgeWethDepositEventArgs as WethDepositArgs, + // TestUniswapV2BridgeWethWithdrawEventArgs as WethWithdrawArgs, } from './wrappers'; -blockchainTests.resets.only('UniswapV2 unit tests', env => { +blockchainTests.resets('UniswapV2 unit tests', env => { const FROM_TOKEN_DECIMALS = 6; const TO_TOKEN_DECIMALS = 18; const FROM_TOKEN_BASE = new BigNumber(10).pow(FROM_TOKEN_DECIMALS); @@ -46,23 +50,24 @@ blockchainTests.resets.only('UniswapV2 unit tests', env => { }); }); - describe('bridgeTransferFrom()', () => { + describe.only('bridgeTransferFrom()', () => { interface TransferFromOpts { toTokenAddress: string; fromTokenAddress: string; toAddress: string; // Amount to pass into `bridgeTransferFrom()` amount: BigNumber; - // Amount to convert in `trade()`. - fillAmount: BigNumber; // Token balance of the bridge. fromTokenBalance: BigNumber; + // Router reverts with this reason + routerRevertReason: string; } interface TransferFromResult { opts: TransferFromOpts; result: string; logs: DecodedLogs; + blocktime: number; } function createTransferFromOpts(opts?: Partial): TransferFromOpts { @@ -72,29 +77,32 @@ blockchainTests.resets.only('UniswapV2 unit tests', env => { toTokenAddress: constants.NULL_ADDRESS, amount, toAddress: randomAddress(), - fillAmount: getRandomPortion(amount), fromTokenBalance: getRandomInteger(1, FROM_TOKEN_BASE.times(100)), + routerRevertReason: '', ...opts, }; } async function transferFromAsync(opts?: Partial): Promise { const _opts = createTransferFromOpts(opts); - const callData = { value: new BigNumber(_opts.fillAmount) }; + // Create the "from" token. const createFromTokenFn = testContract.createToken(_opts.fromTokenAddress); - _opts.fromTokenAddress = await createFromTokenFn.callAsync(callData); - await createFromTokenFn.awaitTransactionSuccessAsync(callData); + _opts.fromTokenAddress = await createFromTokenFn.callAsync(); + await createFromTokenFn.awaitTransactionSuccessAsync(); // Create the "to" token. const createToTokenFn = testContract.createToken(_opts.toTokenAddress); - _opts.toTokenAddress = await createToTokenFn.callAsync(callData); - await createToTokenFn.awaitTransactionSuccessAsync(callData); + _opts.toTokenAddress = await createToTokenFn.callAsync(); + await createToTokenFn.awaitTransactionSuccessAsync(); // Set the token balance for the token we're converting from. - await testContract.setTokenBalance(_opts.fromTokenAddress).awaitTransactionSuccessAsync({ - value: new BigNumber(_opts.fromTokenBalance), - }); + await testContract + .setTokenBalance(_opts.fromTokenAddress, _opts.fromTokenBalance) + .awaitTransactionSuccessAsync(); + + // Set revert reason for the router. + await testContract.setRouterRevertReason(_opts.routerRevertReason).awaitTransactionSuccessAsync(); // Call bridgeTransferFrom(). const bridgeTransferFromFn = testContract.bridgeTransferFrom( @@ -110,11 +118,12 @@ blockchainTests.resets.only('UniswapV2 unit tests', env => { hexUtils.leftPad(_opts.fromTokenAddress), ); const result = await bridgeTransferFromFn.callAsync(); - const { logs } = await bridgeTransferFromFn.awaitTransactionSuccessAsync(); + const receipt = await bridgeTransferFromFn.awaitTransactionSuccessAsync(); return { opts: _opts, result, - logs: (logs as any) as DecodedLogs, + logs: (receipt.logs as any) as DecodedLogs, + blocktime: await env.web3Wrapper.getBlockTimestampAsync(receipt.blockNumber), }; } @@ -123,37 +132,68 @@ blockchainTests.resets.only('UniswapV2 unit tests', env => { expect(result).to.eq(AssetProxyId.ERC20Bridge); }); - it('transfers from one token to another', async () => { - const { opts, result, logs } = await transferFromAsync(); - expect(result).to.eq(AssetProxyId.ERC20Bridge, 'asset proxy id'); - const transfers = filterLogsToArguments( - logs, - UniswapV2BridgeEvents.ERC20BridgeTransfer, - ); - expect(transfers.length).to.eq(1); - expect(transfers[0].inputToken).to.eq(opts.fromTokenAddress, 'input token address'); - expect(transfers[0].outputToken).to.eq(opts.toTokenAddress, 'output token address'); - expect(transfers[0].to).to.eq(opts.toAddress, 'recipient address'); - expect(transfers[0].inputTokenAmount).to.bignumber.eq(opts.fromTokenBalance, 'input token amount'); - expect(transfers[0].outputTokenAmount).to.bignumber.eq(opts.amount, 'output token amount'); - }); - it.skip('transfers when both tokens are the same', async () => { - const address = randomAddress(); + it('performs transfer when both tokens are the same', async () => { + const createTokenFn = testContract.createToken(constants.NULL_ADDRESS); + const tokenAddress = await createTokenFn.callAsync(); + await createTokenFn.awaitTransactionSuccessAsync(); + const { opts, result, logs } = await transferFromAsync({ - fromTokenAddress: address, - toTokenAddress: address, + fromTokenAddress: tokenAddress, + toTokenAddress: tokenAddress, }); expect(result).to.eq(AssetProxyId.ERC20Bridge, 'asset proxy id'); - const transfers = filterLogsToArguments( - logs, - UniswapV2BridgeEvents.ERC20BridgeTransfer, - ); + const transfers = filterLogsToArguments(logs, ContractEvents.TokenTransfer); + expect(transfers.length).to.eq(1); - expect(transfers[0].inputToken).to.eq(opts.fromTokenAddress, 'input token address'); - expect(transfers[0].outputToken).to.eq(opts.toTokenAddress, 'output token address'); + expect(transfers[0].token).to.eq(tokenAddress, 'input token address'); + expect(transfers[0].from).to.eq(testContract.address); expect(transfers[0].to).to.eq(opts.toAddress, 'recipient address'); - expect(transfers[0].inputTokenAmount).to.bignumber.eq(opts.fromTokenBalance, 'input token amount'); - expect(transfers[0].outputTokenAmount).to.bignumber.eq(opts.amount, 'output token amount'); + expect(transfers[0].amount).to.bignumber.eq(opts.amount, 'amount'); + }); + + describe('token -> token', async () => { + it('calls UniswapV2Router01.swapExactTokensForTokens()', async () => { + const { opts, result, logs, blocktime } = await transferFromAsync(); + expect(result).to.eq(AssetProxyId.ERC20Bridge, 'asset proxy id'); + const transfers = filterLogsToArguments( + logs, + ContractEvents.SwapExactTokensForTokens, + ); + + expect(transfers.length).to.eq(1); + expect(transfers[0].toTokenAddress).to.eq(opts.toTokenAddress, 'output token address'); + expect(transfers[0].to).to.eq(opts.toAddress, 'recipient address'); + expect(transfers[0].amountIn).to.bignumber.eq(opts.fromTokenBalance, 'input token amount'); + expect(transfers[0].amountOutMin).to.bignumber.eq(opts.amount, 'output token amount'); + expect(transfers[0].deadline).to.bignumber.eq(blocktime, 'deadline'); + }); + + it('sets allowance for "from" token', async () => { + const { opts, logs } = await transferFromAsync(); + const approvals = filterLogsToArguments(logs, ContractEvents.TokenApprove); + const routerAddress = await testContract.getRouterAddress().callAsync(); + expect(approvals.length).to.eq(1); + expect(approvals[0].spender).to.eq(routerAddress); + expect(approvals[0].allowance).to.bignumber.eq(opts.fromTokenBalance); + }); + + it('sets allowance for "from" token on subsequent calls', async () => { + const { opts } = await transferFromAsync(); + const { logs } = await transferFromAsync(opts); + const approvals = filterLogsToArguments(logs, ContractEvents.TokenApprove); + const routerAddress = await testContract.getRouterAddress().callAsync(); + expect(approvals.length).to.eq(1); + expect(approvals[0].spender).to.eq(routerAddress); + expect(approvals[0].allowance).to.bignumber.eq(opts.fromTokenBalance); + }); + + it('fails if the router fails', async () => { + const revertReason = 'FOOBAR'; + const tx = transferFromAsync({ + routerRevertReason: revertReason, + }); + return expect(tx).to.eventually.be.rejectedWith(revertReason); + }); }); }); });