Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Max approval erc20s #971

Merged
merged 12 commits into from
Oct 11, 2023
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

# 3.0.1

### Upgrade steps

Update `BackingManager`, both `RevenueTraders` (rTokenTrader/rsrTrader), and call `Broker.setBatchTradeImplementation()` passing in the new `GnosisTrade` address.

# 3.0.0

### Upgrade Steps
Expand Down
32 changes: 32 additions & 0 deletions contracts/libraries/Allowance.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// SPDX-License-Identifier: BlueOak-1.0.0
pragma solidity 0.8.19;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

library AllowanceLib {
/// An approve helper that:
/// 1. Sets initial allowance to 0
/// 2. Tries to set the provided allowance
/// 3. Falls back to setting a maximum allowance, if (2) fails
/// Context: Some new-age ERC20s think it's a good idea to revert for allowances
/// that are > 0 but < type(uint256).max.
function safeApproveFallbackToMax(
IERC20 token,
address spender,
uint256 value
) internal {
// 1. Set initial allowance to 0
require(token.approve(spender, 0), "failed to set 0 allowance");
pmckelvy1 marked this conversation as resolved.
Show resolved Hide resolved

if (value == 0) return;

// 2. Try to set the provided allowance
bool success; // bool success = false;
try token.approve(spender, value) returns (bool _success) {
success = _success;
} catch {}

// 3. Fall-back to setting a maximum allowance
if (!success) token.approve(spender, type(uint256).max);
}
}
9 changes: 7 additions & 2 deletions contracts/p0/mixins/Trading.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "../../interfaces/IBroker.sol";
import "../../interfaces/IMain.sol";
import "../../interfaces/ITrade.sol";
import "../../libraries/Allowance.sol";
import "../../libraries/Fixed.sol";
import "./Rewardable.sol";

Expand Down Expand Up @@ -68,8 +69,12 @@ abstract contract TradingP0 is RewardableP0, ITrading {
IBroker broker = main.broker();
assert(address(trades[req.sell.erc20()]) == address(0));

req.sell.erc20().safeApprove(address(broker), 0);
req.sell.erc20().safeApprove(address(broker), req.sellAmount);
// Set allowance via custom approval -- first sets allowance to 0, then sets allowance
// to either the requested amount or the maximum possible amount, if that fails.
//
// Context: wcUSDCv3 has a non-standard approve() function that reverts if the approve
// amount is > 0 and < type(uint256).max.
AllowanceLib.safeApproveFallbackToMax(req.sell.erc20(), address(broker), req.sellAmount);

trade = broker.openTrade(kind, req, prices);
trades[req.sell.erc20()] = trade;
Expand Down
9 changes: 7 additions & 2 deletions contracts/p1/mixins/Trading.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/utils/Multicall.sol";
import "../../interfaces/ITrade.sol";
import "../../interfaces/ITrading.sol";
import "../../libraries/Allowance.sol";
import "../../libraries/Fixed.sol";
import "./Component.sol";
import "./RewardableLib.sol";
Expand Down Expand Up @@ -120,8 +121,12 @@ abstract contract TradingP1 is Multicall, ComponentP1, ReentrancyGuardUpgradeabl
IERC20 sell = req.sell.erc20();
assert(address(trades[sell]) == address(0));

IERC20Upgradeable(address(sell)).safeApprove(address(broker), 0);
IERC20Upgradeable(address(sell)).safeApprove(address(broker), req.sellAmount);
// Set allowance via custom approval -- first sets allowance to 0, then sets allowance
// to either the requested amount or the maximum possible amount, if that fails.
//
// Context: wcUSDCv3 has a non-standard approve() function that reverts if the approve
// amount is > 0 and < type(uint256).max.
AllowanceLib.safeApproveFallbackToMax(req.sell.erc20(), address(broker), req.sellAmount);
pmckelvy1 marked this conversation as resolved.
Show resolved Hide resolved

trade = broker.openTrade(kind, req, prices);
trades[sell] = trade;
Expand Down
10 changes: 7 additions & 3 deletions contracts/plugins/trading/GnosisTrade.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.19;
import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import "@openzeppelin/contracts/utils/math/Math.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
import "../../libraries/Allowance.sol";
import "../../libraries/Fixed.sol";
import "../../interfaces/IBroker.sol";
import "../../interfaces/IGnosis.sol";
Expand Down Expand Up @@ -130,9 +131,12 @@ contract GnosisTrade is ITrade {

// == Interactions ==

// Set allowance (two safeApprove calls to support USDT)
IERC20Upgradeable(address(sell)).safeApprove(address(gnosis), 0);
IERC20Upgradeable(address(sell)).safeApprove(address(gnosis), initBal);
// Set allowance via custom approval -- first sets allowance to 0, then sets allowance
// to either the requested amount or the maximum possible amount, if that fails.
//
// Context: wcUSDCv3 has a non-standard approve() function that reverts if the approve
// amount is > 0 and < type(uint256).max.
AllowanceLib.safeApproveFallbackToMax(sell, address(gnosis), initBal);

auctionId = gnosis.initiateAuction(
sell,
Expand Down