From b38378187082483ba867ad0d4af3c5cc769035e7 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 30 Sep 2019 14:02:10 -0700 Subject: [PATCH] `@0x/contracts-asset-proxy`: Getting around stack issues. --- .../contracts/src/bridges/UniswapBridge.sol | 75 +-- .../{IUniswap.sol => IUniswapExchange.sol} | 18 +- .../interfaces/IUniswapExchangeFactory.sol | 32 ++ .../contracts/test/TestUniswapBridge.sol | 450 ++++++++++++------ 4 files changed, 376 insertions(+), 199 deletions(-) rename contracts/asset-proxy/contracts/src/interfaces/{IUniswap.sol => IUniswapExchange.sol} (86%) create mode 100644 contracts/asset-proxy/contracts/src/interfaces/IUniswapExchangeFactory.sol diff --git a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol index bcb0a6ce09..68e140894e 100644 --- a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol @@ -20,21 +20,31 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-exchange/contracts/src/interfaces/IWallet.sol"; -import "../interfaces/IUniswap.sol"; +import "../interfaces/IUniswapExchangeFactory.sol"; +import "../interfaces/IUniswapExchange.sol"; import "./ERC20Bridge.sol"; // solhint-disable space-after-comma -contract UniswaBridge is +contract UniswapBridge is ERC20Bridge, IWallet { bytes4 private constant LEGACY_WALLET_MAGIC_VALUE = 0xb0671381; /* Mainnet addresses */ - address constant public UNISWAP_EXCHANGE_FACTORY_ADDRESS = address(0); + address constant public UNISWAP_EXCHANGE_FACTORY_ADDRESS = 0xc0a47dFe034B400B47bDaD5FecDa2621de6c4d95; address constant public WETH_ADDRESS = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; + // Struct to hold `withdrawTo()` local variables in memory and to avoid + // stack overflows. + struct WithdrawToState { + IUniswapExchange exchange; + uint256 fromTokenBalance; + IEtherToken weth; + } + /// @dev Whether we've granted an allowance to a spender for a token. mapping (address => mapping (address => bool)) private _hasAllowance; @@ -51,37 +61,41 @@ contract UniswaBridge is address /* from */, address to, uint256 amount, - bytes calldata bridgeData, + bytes calldata bridgeData ) external returns (bytes4 success) { + // State memory object to avoid stack overflows. + WithdrawToState memory state; // Decode the bridge data to get the `fromTokenAddress`. (address fromTokenAddress) = abi.decode(bridgeData, (address)); // Just transfer the tokens if they're the same. if (fromTokenAddress == toTokenAddress) { - IERC20Token(fromToken).transfer(to, amount); + IERC20Token(fromTokenAddress).transfer(to, amount); return BRIDGE_SUCCESS; } // Get the exchange for the token pair. - IUniswapExchange exchange = _getUniswapExchangeForTokenPair( + state.exchange = _getUniswapExchangeForTokenPair( fromTokenAddress, toTokenAddress ); // Grant an allowance to the exchange. - _grantAllowanceForTokens(address(exchange), [fromTokenAddress, toTokenAddress]); + _grantAllowanceForTokens(address(state.exchange), [fromTokenAddress, toTokenAddress]); // Get our balance of `fromTokenAddress` token. - uint256 fromTokenBalance = IERC20Token(fromToken).balanceOf(address(this)); + state.fromTokenBalance = IERC20Token(fromTokenAddress).balanceOf(address(this)); + // Get the weth contract. + state.weth = _getWethContract(); // Convert from WETH to a token. - if (fromTokenAddress == address(weth)) { + if (fromTokenAddress == address(state.weth)) { // Unwrap the WETH. - _getWethContract().withdraw(fromTokenBalance); + state.weth.withdraw(state.fromTokenBalance); // Buy as much of `toTokenAddress` token with ETH as possible and // transfer it to `to`. - exchange.ethToTokenTransferInput.value(fromTokenBalance)( + state.exchange.ethToTokenTransferInput.value(state.fromTokenBalance)( // No minimum buy amount. 0, // Expires after this block. @@ -91,18 +105,18 @@ contract UniswaBridge is ); // Convert from a token to WETH. - } else if (toTokenAddress == address(weth)) { + } else if (toTokenAddress == address(state.weth)) { // Buy as much ETH with `toTokenAddress` token as possible. - uint256 ethBought = exchange.tokenToEthSwapInput( + uint256 ethBought = state.exchange.tokenToEthSwapInput( // Sell all tokens we hold. - fromTokenBalance, + state.fromTokenBalance, + // No minimum buy amount. + 0, // Expires after this block. - block.timestamp, - // Recipient is `to`. - to + block.timestamp ); // Wrap the ETH. - _getWethContract().deposit.value(ethBought)(); + state.weth.deposit.value(ethBought)(); // Transfer the WETH to `to`. IERC20Token(toTokenAddress).transfer(to, ethBought); @@ -110,9 +124,9 @@ contract UniswaBridge is } else { // Buy as much `toTokenAddress` token with `fromTokenAddress` token // and transfer it to `to`. - exchange.tokenToTokenTransferInput( + state.exchange.tokenToTokenTransferInput( // Sell all tokens we hold. - fromTokenBalance, + state.fromTokenBalance, // No minimum buy amount. 0, // No minimum intermediate ETH buy amount. @@ -147,9 +161,9 @@ contract UniswaBridge is function _getWethContract() internal view - returns (IERC20Token token) + returns (IEtherToken token) { - return IERC20Token(WETH_ADDRESS); + return IEtherToken(WETH_ADDRESS); } /// @dev Overridable way to get the uniswap exchange factory contract. @@ -159,17 +173,17 @@ contract UniswaBridge is view returns (IUniswapExchangeFactory factory) { - return IUniswapExchangeFactory(ETH2DAI_ADDRESS); + return IUniswapExchangeFactory(UNISWAP_EXCHANGE_FACTORY_ADDRESS); } - /// @dev Grants an unlimited allowance to `spender` for `fromTokenAddress` - /// and `toTokenAddress` tokens, if they're not WETH and we haven't - /// already granted `spender` an allowance. + /// @dev Grants an unlimited allowance to `spender` for the tokens passed, + /// if they're not WETH and we haven't already granted `spender` an + /// allowance. /// @param spender The spender being granted an aloowance. /// @param tokenAddresses Array of token addresses. function _grantAllowanceForTokens( address spender, - address[2] memory tokenAddresses, + address[2] memory tokenAddresses ) private { @@ -210,9 +224,8 @@ contract UniswaBridge is view returns (IUniswapExchange exchange) { - address exchangeAddress = _getUniswapExchangeFactoryContract() - .getExchange(tokenAddress); - require(exchangeAddress != address(0), "NO_UNISWAP_EXCHANGE_FOR_TOKEN"); - return IUniswapExchange(exchangeAddress); + exchange = _getUniswapExchangeFactoryContract().getExchange(tokenAddress); + require(address(exchange) != address(0), "NO_UNISWAP_EXCHANGE_FOR_TOKEN"); + return exchange; } } diff --git a/contracts/asset-proxy/contracts/src/interfaces/IUniswap.sol b/contracts/asset-proxy/contracts/src/interfaces/IUniswapExchange.sol similarity index 86% rename from contracts/asset-proxy/contracts/src/interfaces/IUniswap.sol rename to contracts/asset-proxy/contracts/src/interfaces/IUniswapExchange.sol index c4035f93f6..f11b2304de 100644 --- a/contracts/asset-proxy/contracts/src/interfaces/IUniswap.sol +++ b/contracts/asset-proxy/contracts/src/interfaces/IUniswapExchange.sol @@ -19,7 +19,6 @@ pragma solidity ^0.5.9; -// solhint-disable func-param-name-mixedcase interface IUniswapExchange { /// @dev Buys at least `minTokensBought` tokens with ETH and transfer them @@ -30,7 +29,7 @@ interface IUniswapExchange { /// @return tokensBought Amount of tokens bought. function ethToTokenTransferInput( uint256 minTokensBought, - uint64 deadline, + uint256 deadline, address recipient ) external @@ -45,7 +44,7 @@ interface IUniswapExchange { function tokenToEthSwapInput( uint256 tokensSold, uint256 minEthBought, - uint64 deadline + uint256 deadline ) external payable @@ -63,21 +62,10 @@ interface IUniswapExchange { uint256 tokensSold, uint256 minTokensBought, uint256 minEthBought, - uint64 deadline, + uint256 deadline, address recipient, address toTokenAddress ) external returns (uint256 tokensBought); } - - -interface IUniswapExchangeFactory { - - /// @dev Get the exchange for a token. - /// @param tokenAddress The address of the token contract. - function getExchange(address tokenAddress) - external - view - returns (IUniswapExchange); -} diff --git a/contracts/asset-proxy/contracts/src/interfaces/IUniswapExchangeFactory.sol b/contracts/asset-proxy/contracts/src/interfaces/IUniswapExchangeFactory.sol new file mode 100644 index 0000000000..c175f55326 --- /dev/null +++ b/contracts/asset-proxy/contracts/src/interfaces/IUniswapExchangeFactory.sol @@ -0,0 +1,32 @@ +/* + + 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; + +import "./IUniswapExchange.sol"; + + +interface IUniswapExchangeFactory { + + /// @dev Get the exchange for a token. + /// @param tokenAddress The address of the token contract. + function getExchange(address tokenAddress) + external + view + returns (IUniswapExchange); +} diff --git a/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol b/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol index 943cda072b..a6ab535f82 100644 --- a/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol +++ b/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol @@ -20,101 +20,14 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; +import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../src/bridges/UniswapBridge.sol"; -import "../src/interfaces/IUniswap.sol"; +import "../src/interfaces/IUniswapExchangeFactory.sol"; +import "../src/interfaces/IUniswapExchange.sol"; -// solhint-disable no-simple-event-func-name -/// @dev Interface that allows `TestToken` to call functions on the -/// `TestUniswapBridge` contract. -interface ITestEventRaiser { +contract TestEventsRaiser { - function raiseTokenTransferEvent( - address from, - address to, - uint256 amount - ) - external; - - function raiseTokenApproveEvent( - address spender, - uint256 allowance - ) - external; - - function raiseWethDeposit( - uint256 amount - ) - external; - - function raiseWethWithdraw( - uint256 amount - ) - external; -} - - -/// @dev A minimalist ERC20/WETH token. -contract TestToken { - - mapping (address => uint256) public balances; - - /// @dev Just calls `raiseTokenTransferEvent()` on the caller. - function transfer(address to, uint256 amount) - external - returns (bool) - { - ITestEventRaiser(msg.sender).raiseTokenTransferEvent(msg.sender, to, amount); - return true; - } - - /// @dev Just calls `raiseTokenApproveEvent()` on the caller. - function approve(address spender, uint256 allowance) - external - returns (bool) - { - ITestEventRaiser(msg.sender).raiseTokenApproveEvent(spender, allowance); - return true; - } - - /// @dev Set the balance for `owner`. - function setBalance(address owner, uint256 balance) - external - { - balances[owner] = balance; - } - - // @dev `IWETH.deposit()` that just calls `raiseWethDeposit()` on the caller. - function deposit() - external - payable - { - ITestEventRaiser(msg.sender).raiseWethDeposit(msg.value); - } - - // @dev `IWETH.withdraw()` that just calls `raiseWethWithdraw()` on the caller. - function withdraw(uint256 amount) - external - { - ITestEventRaiser(msg.sender).raiseWethWithdraw(amount); - } - - /// @dev Retrieve the balance for `owner`. - function balanceOf(address owner) - external - view - returns (uint256) - { - return balances[owner]; - } -} - - -/// @dev UniswapBridge overridden to mock tokens and implement IUniswap. -contract TestUniswapBridge is - IUniswap, - UniswapBridge -{ event SellAllAmount( address sellToken, uint256 sellTokenAmount, @@ -142,50 +55,76 @@ contract TestUniswapBridge is uint256 amount ); - TestToken public wethToken = new TestToken(); - TestToken public daiToken = new TestToken(); - string private _nextRevertReason; - uint256 private _nextFillAmount; + event EthToTokenTransferInput( + uint256 minTokensBought, + uint256 deadline, + address recipient + ); - /// @dev Set token balances for this contract. - function setTokenBalances(uint256 wethBalance, uint256 daiBalance) - external - { - wethToken.setBalance(address(this), wethBalance); - daiToken.setBalance(address(this), daiBalance); - } + event TokenToEthSwapInput( + uint256 tokensSold, + uint256 minEthBought, + uint256 deadline + ); - /// @dev Set the behavior for `IUniswap.sellAllAmount()`. - function setFillBehavior(string calldata revertReason, uint256 fillAmount) - external - { - _nextRevertReason = revertReason; - _nextFillAmount = fillAmount; - } + event TokenToTokenTransferInput( + uint256 tokensSold, + uint256 minTokensBought, + uint256 minEthBought, + uint256 deadline, + address recipient, + address toTokenAddress + ); - /// @dev Implementation of `IUniswap.sellAllAmount()` - function sellAllAmount( - address sellTokenAddress, - uint256 sellTokenAmount, - address buyTokenAddress, - uint256 minimumFillAmount + function raiseEthToTokenTransferInput( + uint256 minTokensBought, + uint256 deadline, + address recipient ) external - returns (uint256 fillAmount) { - emit SellAllAmount( - sellTokenAddress, - sellTokenAmount, - buyTokenAddress, - minimumFillAmount + emit EthToTokenTransferInput( + minTokensBought, + deadline, + recipient ); - if (bytes(_nextRevertReason).length != 0) { - revert(_nextRevertReason); - } - return _nextFillAmount; } - function raiseTokenTransferEvent( + function raiseTokenToEthSwapInput( + uint256 tokensSold, + uint256 minEthBought, + uint256 deadline + ) + external + { + emit TokenToEthSwapInput( + tokensSold, + minEthBought, + deadline + ); + } + + function raiseTokenToTokenTransferInput( + uint256 tokensSold, + uint256 minTokensBought, + uint256 minEthBought, + uint256 deadline, + address recipient, + address toTokenAddress + ) + external + { + emit TokenToTokenTransferInput( + tokensSold, + minTokensBought, + minEthBought, + deadline, + recipient, + toTokenAddress + ); + } + + function raiseTokenTransfer( address from, address to, uint256 amount @@ -200,53 +139,258 @@ contract TestUniswapBridge is ); } - function raiseTokenApproveEvent( - address spender, - uint256 allowance - ) + function raiseTokenApprove(address spender, uint256 allowance) external { - emit TokenApprove( - spender, - allowance + emit TokenApprove(spender, allowance); + } + + function raiseWethDeposit(uint256 amount) + external + { + emit WethDeposit(amount); + } + + function raiseWethWithdraw(uint256 amount) + external + { + emit WethWithdraw(amount); + } +} + +/// @dev A minimalist ERC20/WETH token. +contract TestToken { + + using LibSafeMath for uint256; + + mapping (address => uint256) public balances; + + /// @dev Calls `raiseTokenTransfer()` on the caller. + function transfer(address to, uint256 amount) + external + returns (bool) + { + TestEventsRaiser(msg.sender).raiseTokenTransfer(msg.sender, to, amount); + balances[msg.sender] = balances[msg.sender].safeSub(amount); + balances[to] = balances[to].safeAdd(amount); + return true; + } + + /// @dev Just calls `raiseTokenApprove()` on the caller. + function approve(address spender, uint256 allowance) + external + returns (bool) + { + TestEventsRaiser(msg.sender).raiseTokenApprove(spender, allowance); + return true; + } + + /// @dev Set the balance for `owner`. + function setBalance(address owner, uint256 balance) + external + payable + { + balances[owner] = balance; + } + + /// @dev `IWETH.deposit()` that increases balances and calls + /// `raiseWethDeposit()` on the caller. + function deposit() + external + payable + { + 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 + { + balances[msg.sender] = balances[msg.sender].safeSub(amount); + msg.sender.transfer(amount); + TestEventsRaiser(msg.sender).raiseWethWithdraw(amount); + } + + /// @dev Retrieve the balance for `owner`. + function balanceOf(address owner) + external + view + returns (uint256) + { + return balances[owner]; + } +} + + +contract TestExchange is + IUniswapExchange +{ + address public tokenAddress; + string private _nextRevertReason; + uint256 private _nextFillAmount; + + constructor(address _tokenAddress) public { + tokenAddress = _tokenAddress; + } + + function setFillBehavior( + string calldata revertReason, + uint256 fillAmount + ) + external + payable + { + _nextRevertReason = revertReason; + _nextFillAmount = fillAmount; + } + + function ethToTokenTransferInput( + uint256 minTokensBought, + uint256 deadline, + address recipient + ) + external + payable + returns (uint256 tokensBought) + { + TestEventsRaiser(msg.sender).raiseEthToTokenTransferInput( + minTokensBought, + deadline, + recipient + ); + _revertIfReasonExists(); + return _nextFillAmount; + } + + function tokenToEthSwapInput( + uint256 tokensSold, + uint256 minEthBought, + uint256 deadline + ) + external + payable + returns (uint256 ethBought) + { + TestEventsRaiser(msg.sender).raiseTokenToEthSwapInput( + tokensSold, + minEthBought, + deadline + ); + _revertIfReasonExists(); + return _nextFillAmount; + } + + function tokenToTokenTransferInput( + uint256 tokensSold, + uint256 minTokensBought, + uint256 minEthBought, + uint256 deadline, + address recipient, + address toTokenAddress + ) + external + returns (uint256 tokensBought) + { + TestEventsRaiser(msg.sender).raiseTokenToTokenTransferInput( + tokensSold, + minTokensBought, + minEthBought, + deadline, + recipient, + toTokenAddress + ); + _revertIfReasonExists(); + return _nextFillAmount; + } + + function _revertIfReasonExists() + private + { + if (bytes(_nextRevertReason).length != 0) { + revert(_nextRevertReason); + } + } +} + + +/// @dev UniswapBridge overridden to mock tokens and implement IUniswapExchangeFactory. +contract TestUniswapBridge is + IUniswapExchangeFactory, + TestEventsRaiser, + UniswapBridge +{ + + TestToken public wethToken = new TestToken(); + // Token address to TestToken instance. + mapping (address => TestToken) private _testTokens; + // Token address to TestExchange instance. + mapping (address => TestExchange) private _testExchanges; + + /// @dev Set token balances for this contract. + function setTokenBalances(address tokenAddress, uint256 balance) + external + { + TestToken token = _testTokens[tokenAddress]; + // Create the token if it doesn't exist. + if (address(token) == address(0)) { + _testTokens[tokenAddress] = token = new TestToken(); + } + token.setBalance(address(this), balance); + } + + /// @dev Set the behavior for a fill on a uniswap exchange. + function setExchangeFillBehavior( + address exchangeAddress, + string calldata revertReason, + uint256 fillAmount + ) + external + payable + { + createExchange(exchangeAddress).setFillBehavior.value(msg.value)( + revertReason, + fillAmount ); } - /// @dev Retrieves the allowances of the test tokens. - function getUniswapTokenAllowances() + /// @dev Create an exchange for a token. + function createExchange(address tokenAddress) + public + returns (TestExchange exchangeAddress) + { + TestExchange exchange = _testExchanges[tokenAddress]; + if (address(exchange) == address(0)) { + _testExchanges[tokenAddress] = exchange = new TestExchange(tokenAddress); + } + return exchange; + } + + /// @dev `IUniswapExchangeFactory.getExchange` + function getExchange(address tokenAddress) external view - returns (uint256 wethAllowance, uint256 daiAllowance) + returns (IUniswapExchange) { - wethAllowance = wethToken.allowances(address(this), address(this)); - daiAllowance = daiToken.allowances(address(this), address(this)); - return (wethAllowance, daiAllowance); + return IUniswapExchange(_testExchanges[tokenAddress]); } // @dev Use `wethToken`. function _getWethContract() internal view - returns (IERC20Token) + returns (IEtherToken) { - return IERC20Token(address(wethToken)); - } - - // @dev Use `daiToken`. - function _getDaiContract() - internal - view - returns (IERC20Token) - { - return IERC20Token(address(daiToken)); + return IEtherToken(address(wethToken)); } // @dev This contract will double as the Uniswap contract. - function _getUniswapContract() + function _getUniswapExchangeFactoryContract() internal view - returns (IUniswap) + returns (IUniswapExchangeFactory) { - return IUniswap(address(this)); + return IUniswapExchangeFactory(address(this)); } }