diff --git a/contracts/asset-proxy/CHANGELOG.json b/contracts/asset-proxy/CHANGELOG.json index 4d2c5c3dd3..637623b18c 100644 --- a/contracts/asset-proxy/CHANGELOG.json +++ b/contracts/asset-proxy/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "version": "3.3.0", + "changes": [ + { + "note": "Use `approveIfBelowMax()` in DEX bridges for for approvals." + } + ] + }, { "timestamp": 1583220306, "version": "3.2.5", diff --git a/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol b/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol index 9bbe57f9b1..56a6ce388b 100644 --- a/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/CurveBridge.sol @@ -61,7 +61,7 @@ contract CurveBridge is address fromTokenAddress = exchange.underlying_coins(fromCoinIdx); require(toTokenAddress != fromTokenAddress, "CurveBridge/INVALID_PAIR"); // Grant an allowance to the exchange to spend `fromTokenAddress` token. - LibERC20Token.approve(fromTokenAddress, address(exchange), uint256(-1)); + LibERC20Token.approveIfBelowMax(fromTokenAddress, address(exchange)); // Try to sell all of this contract's `fromTokenAddress` token balance. if (version == 0) { diff --git a/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol b/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol index b3b84d8d96..ff01a67376 100644 --- a/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/Eth2DaiBridge.sol @@ -57,7 +57,7 @@ contract Eth2DaiBridge is IEth2Dai exchange = IEth2Dai(_getEth2DaiAddress()); // Grant an allowance to the exchange to spend `fromTokenAddress` token. - LibERC20Token.approve(fromTokenAddress, address(exchange), uint256(-1)); + LibERC20Token.approveIfBelowMax(fromTokenAddress, address(exchange)); // 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 2a98663897..c9acfb9286 100644 --- a/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/KyberBridge.sol @@ -109,7 +109,7 @@ 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.approve(state.fromTokenAddress, address(state.kyber), uint256(-1)); + LibERC20Token.approveIfBelowMax(state.fromTokenAddress, address(state.kyber)); } 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 3d0d596273..936191e128 100644 --- a/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol +++ b/contracts/asset-proxy/contracts/src/bridges/UniswapBridge.sol @@ -168,7 +168,7 @@ contract UniswapBridge is function _grantExchangeAllowance(IUniswapExchange exchange, address tokenAddress) private { - LibERC20Token.approve(tokenAddress, address(exchange), uint256(-1)); + LibERC20Token.approveIfBelowMax(tokenAddress, address(exchange)); } /// @dev Retrieves the uniswap exchange for a given token pair. diff --git a/contracts/asset-proxy/contracts/test/TestEth2DaiBridge.sol b/contracts/asset-proxy/contracts/test/TestEth2DaiBridge.sol index 859af2c1af..ee249278b1 100644 --- a/contracts/asset-proxy/contracts/test/TestEth2DaiBridge.sol +++ b/contracts/asset-proxy/contracts/test/TestEth2DaiBridge.sol @@ -110,6 +110,10 @@ contract TestToken { return true; } + function allowance(address, address) external view returns (uint256) { + return 0; + } + /// @dev Retrieve the balance for `owner`. function balanceOf(address owner) external diff --git a/contracts/asset-proxy/contracts/test/TestKyberBridge.sol b/contracts/asset-proxy/contracts/test/TestKyberBridge.sol index 7c6e28f0e8..c8ce0b2b26 100644 --- a/contracts/asset-proxy/contracts/test/TestKyberBridge.sol +++ b/contracts/asset-proxy/contracts/test/TestKyberBridge.sol @@ -110,6 +110,10 @@ contract TestToken { return _testContract.wethDeposit.value(msg.value)(msg.sender); } + function allowance(address, address) external view returns (uint256) { + return 0; + } + function balanceOf(address owner) external view diff --git a/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol b/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol index f295a939f7..bc9b53b650 100644 --- a/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol +++ b/contracts/asset-proxy/contracts/test/TestUniswapBridge.sol @@ -224,6 +224,10 @@ contract TestToken { TestEventsRaiser(msg.sender).raiseWethWithdraw(amount); } + function allowance(address, address) external view returns (uint256) { + return 0; + } + /// @dev Retrieve the balance for `owner`. function balanceOf(address owner) external diff --git a/contracts/erc20/CHANGELOG.json b/contracts/erc20/CHANGELOG.json index 07a60ba9a4..244032d61a 100644 --- a/contracts/erc20/CHANGELOG.json +++ b/contracts/erc20/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "version": "3.2.0", + "changes": [ + { + "note": "Add `LibERC20Token.approveIfBelowMax()`" + } + ] + }, { "timestamp": 1583220306, "version": "3.1.5", diff --git a/contracts/erc20/contracts/src/LibERC20Token.sol b/contracts/erc20/contracts/src/LibERC20Token.sol index 2ba86732b3..4837e97d1f 100644 --- a/contracts/erc20/contracts/src/LibERC20Token.sol +++ b/contracts/erc20/contracts/src/LibERC20Token.sol @@ -47,6 +47,23 @@ library LibERC20Token { _callWithOptionalBooleanResult(token, callData); } + /// @dev Calls `IERC20Token(token).approve()` and sets the allowance to the + /// maximum if the current approval is not already set to the maximum. + /// 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( + address token, + address spender + ) + internal + { + if (IERC20Token(token).allowance(address(this), spender) != uint256(-1)) { + approve(token, spender, uint256(-1)); + } + } + /// @dev Calls `IERC20Token(token).transfer()`. /// Reverts if `false` is returned or if the return /// data length is nonzero and not 32 bytes.