Use SafeERC20.forceApprove in safeIncreaseAllowance and safeDecreaseA…

…llowance (#4260) Co-authored-by: Francisco <[email protected]>
OpenZeppelin · May 25, 2023 · 25edd3c · 25edd3c
1 parent 238d17c
commit 25edd3c
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/.changeset/wild-windows-trade.md b/.changeset/wild-windows-trade.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`SafeERC20`: Refactor `safeDecreaseAllowance` and `safeIncreaseAllowance` to support USDT-like tokens.
diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol
@@ -41,7 +41,7 @@ library SafeERC20 {
      */
     function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal {
         uint256 oldAllowance = token.allowance(address(this), spender);
-        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, oldAllowance + value));
+        forceApprove(token, spender, oldAllowance + value);
     }
 
     /**
@@ -52,7 +52,7 @@ library SafeERC20 {
         unchecked {
             uint256 oldAllowance = token.allowance(address(this), spender);
             require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");
-            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, oldAllowance - value));
+            forceApprove(token, spender, oldAllowance - value);
         }
     }
 

diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js
@@ -182,16 +182,19 @@ contract('SafeERC20', function (accounts) {
         await this.token.$_approve(this.mock.address, spender, 100);
       });
 
-      it('safeApprove can increase approval', async function () {
-        await expectRevert(this.mock.$safeIncreaseAllowance(this.token.address, spender, 10), 'USDT approval failure');
+      it('safeIncreaseAllowance works', async function () {
+        await this.mock.$safeIncreaseAllowance(this.token.address, spender, 10);
+        expect(this.token.allowance(this.mock.address, spender, 90));
       });
 
-      it('safeApprove can decrease approval', async function () {
-        await expectRevert(this.mock.$safeDecreaseAllowance(this.token.address, spender, 10), 'USDT approval failure');
+      it('safeDecreaseAllowance works', async function () {
+        await this.mock.$safeDecreaseAllowance(this.token.address, spender, 10);
+        expect(this.token.allowance(this.mock.address, spender, 110));
       });
 
       it('forceApprove works', async function () {
         await this.mock.$forceApprove(this.token.address, spender, 200);
+        expect(this.token.allowance(this.mock.address, spender, 200));
       });
     });
   });