if value greater than zero check in ERC20TemporaryApproval

OpenZeppelin · Jun 13, 2024 · 17833c7 · 17833c7
1 parent b2d4966
commit 17833c7
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 21 deletions.
diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol
@@ -299,15 +299,13 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
      * Does not emit an {Approval} event.
      */
     function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
-        if (value > 0) {
-            uint256 currentAllowance = allowance(owner, spender);
-            if (currentAllowance != type(uint256).max) {
-                if (currentAllowance < value) {
-                    revert ERC20InsufficientAllowance(spender, currentAllowance, value);
-                }
-                unchecked {
-                    _approve(owner, spender, currentAllowance - value, false);
-                }
+        uint256 currentAllowance = allowance(owner, spender);
+        if (currentAllowance != type(uint256).max) {
+            if (currentAllowance < value) {
+                revert ERC20InsufficientAllowance(spender, currentAllowance, value);
+            }
+            unchecked {
+                _approve(owner, spender, currentAllowance - value, false);
             }
         }
     }

diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol
@@ -89,22 +89,23 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 {
 
         // Check and update (if needed) the temporary allowance + set remaining value
         if (currentTemporaryAllowance > 0) {
+            // All value is covered by the infinite allowance. nothing left to spend, we can return early
             if (currentTemporaryAllowance == type(uint256).max) {
-                // all value is covered by the infinite allowance. nothing left to spend.
-                value = 0;
-            } else {
-                // check how much of the value is covered by the transient allowance
-                uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value);
-                unchecked {
-                    // decrease transient allowance accordingly
-                    _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance);
-                    // update value necessary
-                    value -= spendTemporaryAllowance;
-                }
+                return;
+            }
+            // check how much of the value is covered by the transient allowance
+            uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value);
+            unchecked {
+                // decrease transient allowance accordingly
+                _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance);
+                // update value necessary
+                value -= spendTemporaryAllowance;
             }
         }
         // reduce any remaining value from the persistent allowance
-        super._spendAllowance(owner, spender, value);
+        if (value > 0) {
+            super._spendAllowance(owner, spender, value);
+        }
     }
 
     function _temporaryAllowanceSlot(

diff --git a/...extensions/ERC20TemporaryApproval.test.js → ...ions/draft-ERC20TemporaryApproval.test.js b/...extensions/ERC20TemporaryApproval.test.js → ...ions/draft-ERC20TemporaryApproval.test.js