From c3d4c139361d7722095dc8a46714338d2bc3dd64 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Wed, 4 Mar 2020 21:32:47 -0500 Subject: [PATCH] `@0x/contracts-erc20`: Switch `LibERC20Token.approveIfBelowMax()` to `LibERC20Token.approveIfBelow()`. `@0x/contracts-asset-proxy`: Use `LibERC20Token.approveIfBelow()` for bridge approvals. --- contracts/asset-proxy/CHANGELOG.json | 2 +- .../contracts/src/bridges/CurveBridge.sol | 4 ++-- .../contracts/src/bridges/Eth2DaiBridge.sol | 5 ++--- .../contracts/src/bridges/KyberBridge.sol | 6 +++++- .../contracts/src/bridges/UniswapBridge.sol | 17 +++++++++++++---- contracts/erc20/CHANGELOG.json | 2 +- contracts/erc20/contracts/src/LibERC20Token.sol | 10 ++++++---- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/contracts/asset-proxy/CHANGELOG.json b/contracts/asset-proxy/CHANGELOG.json index 2f8689823e..067a75fa24 100644 --- a/contracts/asset-proxy/CHANGELOG.json +++ b/contracts/asset-proxy/CHANGELOG.json @@ -3,7 +3,7 @@ "version": "3.3.0", "changes": [ { - "note": "Use `approveIfBelowMax()` in DEX bridges for for approvals.", + "note": "Use `LibERC20Token.approveIfBelow()` in DEX bridges for for approvals.", "pr": 2512 }, { diff --git a/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol b/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol index e33234a720..1044dffc7a 100644 --- a/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol @@ -67,9 +67,9 @@ contract CurveBridge is address fromTokenAddress = ICurve(data.curveAddress).underlying_coins(data.fromCoinIdx); require(toTokenAddress != fromTokenAddress, "CurveBridge/INVALID_PAIR"); - // Grant an allowance to the exchange to spend `fromTokenAddress` token. - LibERC20Token.approveIfBelowMax(fromTokenAddress, data.curveAddress); uint256 fromTokenBalance = IERC20Token(fromTokenAddress).balanceOf(address(this)); + // Grant an allowance to the exchange to spend `fromTokenAddress` token. + LibERC20Token.approveIfBelow(fromTokenAddress, data.curveAddress, fromTokenBalance); // Try to sell all of this contract's `fromTokenAddress` token balance. if (data.version == 0) { diff --git a/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol b/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol index 5e8517166e..0bbe784461 100644 --- a/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol @@ -57,10 +57,9 @@ contract Eth2DaiBridge is (address fromTokenAddress) = abi.decode(bridgeData, (address)); IEth2Dai exchange = IEth2Dai(_getEth2DaiAddress()); - // Grant an allowance to the exchange to spend `fromTokenAddress` token. - LibERC20Token.approveIfBelowMax(fromTokenAddress, address(exchange)); - uint256 fromTokenBalance = IERC20Token(fromTokenAddress).balanceOf(address(this)); + // Grant an allowance to the exchange to spend `fromTokenAddress` token. + LibERC20Token.approveIfBelow(fromTokenAddress, address(exchange), fromTokenBalance); // Try to sell all of this contract's `fromTokenAddress` token balance. uint256 boughtAmount = exchange.sellAllAmount( diff --git a/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol b/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol index e377d4fdd5..3e9557d941 100644 --- a/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol @@ -110,7 +110,11 @@ contract KyberBridge is } else if (state.fromTokenAddress != address(state.weth)) { // If the input token is not WETH, grant an allowance to the exchange // to spend them. - LibERC20Token.approveIfBelowMax(state.fromTokenAddress, address(state.kyber)); + LibERC20Token.approveIfBelow( + state.fromTokenAddress, + address(state.kyber), + state.fromTokenBalance + ); } else { // If the input token is WETH, unwrap it and attach it to the call. state.fromTokenAddress = KYBER_ETH_ADDRESS; diff --git a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol index 85e2b62052..35155555d5 100644 --- a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol @@ -110,7 +110,7 @@ contract UniswapBridge is // Convert from a token to WETH. } else if (toTokenAddress == address(state.weth)) { // Grant the exchange an allowance. - _grantExchangeAllowance(state.exchange, fromTokenAddress); + _grantExchangeAllowance(state.exchange, fromTokenAddress, state.fromTokenBalance); // Buy as much ETH with `fromTokenAddress` token as possible. state.boughtAmount = state.exchange.tokenToEthSwapInput( // Sell all tokens we hold. @@ -128,7 +128,7 @@ contract UniswapBridge is // Convert from one token to another. } else { // Grant the exchange an allowance. - _grantExchangeAllowance(state.exchange, fromTokenAddress); + _grantExchangeAllowance(state.exchange, fromTokenAddress, state.fromTokenBalance); // Buy as much `toTokenAddress` token with `fromTokenAddress` token // and transfer it to `to`. state.boughtAmount = state.exchange.tokenToTokenTransferInput( @@ -177,10 +177,19 @@ contract UniswapBridge is /// on behalf of this contract. /// @param exchange The Uniswap token exchange. /// @param tokenAddress The token address for the exchange. - function _grantExchangeAllowance(IUniswapExchange exchange, address tokenAddress) + /// @param minimumAllowance The minimum necessary allowance. + function _grantExchangeAllowance( + IUniswapExchange exchange, + address tokenAddress, + uint256 minimumAllowance + ) private { - LibERC20Token.approveIfBelowMax(tokenAddress, address(exchange)); + LibERC20Token.approveIfBelow( + tokenAddress, + address(exchange), + minimumAllowance + ); } /// @dev Retrieves the uniswap exchange for a given token pair. diff --git a/contracts/erc20/CHANGELOG.json b/contracts/erc20/CHANGELOG.json index a5688131b7..d33d7ae852 100644 --- a/contracts/erc20/CHANGELOG.json +++ b/contracts/erc20/CHANGELOG.json @@ -3,7 +3,7 @@ "version": "3.2.0", "changes": [ { - "note": "Add `LibERC20Token.approveIfBelowMax()`", + "note": "Add `LibERC20Token.approveIfBelow()`", "pr": 2512 } ] diff --git a/contracts/erc20/contracts/src/LibERC20Token.sol b/contracts/erc20/contracts/src/LibERC20Token.sol index 4837e97d1f..651375c62b 100644 --- a/contracts/erc20/contracts/src/LibERC20Token.sol +++ b/contracts/erc20/contracts/src/LibERC20Token.sol @@ -48,18 +48,20 @@ library LibERC20Token { } /// @dev Calls `IERC20Token(token).approve()` and sets the allowance to the - /// maximum if the current approval is not already set to the maximum. + /// maximum if the current approval is not already >= an amount. /// Reverts if `false` is returned or if the return /// data length is nonzero and not 32 bytes. /// @param token The address of the token contract. /// @param spender The address that receives an allowance. - function approveIfBelowMax( + /// @param amount The minimum allowance needed. + function approveIfBelow( address token, - address spender + address spender, + uint256 amount ) internal { - if (IERC20Token(token).allowance(address(this), spender) != uint256(-1)) { + if (IERC20Token(token).allowance(address(this), spender) < amount) { approve(token, spender, uint256(-1)); } }