fix: remove increase and decrease allowance (#75)

yearn · Dec 26, 2023 · 4c7a7a3 · 4c7a7a3
1 parent 28f753f
commit 4c7a7a3
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 163 deletions.
diff --git a/src/TokenizedStrategy.sol b/src/TokenizedStrategy.sol
@@ -1754,59 +1754,6 @@ contract TokenizedStrategy {
         return true;
     }
 
-    /**
-     * @notice Atomically increases the allowance granted to `spender` by the caller.
-     *
-     * @dev This is an alternative to {approve} that can be used as a mitigation for
-     * problems described in {IERC20-approve}.
-     *
-     * Emits an {Approval} event indicating the updated allowance.
-     *
-     * Requirements:
-     *
-     * - `spender` cannot be the zero address.
-     * - cannot give spender over uint256.max allowance
-     *
-     * @param spender the account that will be able to move the senders shares.
-     * @param addedValue the extra amount to add to the current allowance.
-     * @return . a boolean value indicating whether the operation succeeded.
-     */
-    function increaseAllowance(
-        address spender,
-        uint256 addedValue
-    ) external returns (bool) {
-        address owner = msg.sender;
-        _approve(owner, spender, allowance(owner, spender) + addedValue);
-        return true;
-    }
-
-    /**
-     * @notice Atomically decreases the allowance granted to `spender` by the caller.
-     *
-     * @dev This is an alternative to {approve} that can be used as a mitigation for
-     * problems described in {IERC20-approve}.
-     *
-     * Emits an {Approval} event indicating the updated allowance.
-     *
-     * Requirements:
-     *
-     * - `spender` cannot be the zero address.
-     * - `spender` must have allowance for the caller of at least
-     * `subtractedValue`.
-     *
-     * @param spender the account that will be able to move less of the senders shares.
-     * @param subtractedValue the amount to decrease the current allowance by.
-     * @return . a boolean value indicating whether the operation succeeded.
-     */
-    function decreaseAllowance(
-        address spender,
-        uint256 subtractedValue
-    ) external returns (bool) {
-        address owner = msg.sender;
-        _approve(owner, spender, allowance(owner, spender) - subtractedValue);
-        return true;
-    }
-
     /**
      * @dev Moves `amount` of tokens from `from` to `to`.
      *

diff --git a/src/interfaces/ITokenizedStrategy.sol b/src/interfaces/ITokenizedStrategy.sol
@@ -155,18 +155,4 @@ interface ITokenizedStrategy is IERC4626, IERC20Permit {
     function shutdownStrategy() external;
 
     function emergencyWithdraw(uint256 _amount) external;
-
-    /*//////////////////////////////////////////////////////////////
-                           ERC20 ADD ONS
-    //////////////////////////////////////////////////////////////*/
-
-    function decreaseAllowance(
-        address spender,
-        uint256 subtractedValue
-    ) external returns (bool);
-
-    function increaseAllowance(
-        address spender,
-        uint256 addedValue
-    ) external returns (bool);
 }
diff --git a/src/test/ERC20Std.t.sol b/src/test/ERC20Std.t.sol
@@ -62,98 +62,6 @@ contract ERC20BaseTest is Setup {
         assertEq(strategy.allowance(self, account_), amount_);
     }
 
-    function testFuzz_increaseAllowance(
-        address account_,
-        uint256 initialAmount_,
-        uint256 addedAmount_
-    ) public {
-        vm.assume(account_ != address(0) && account_ != address(strategy));
-        initialAmount_ = bound(initialAmount_, 0, type(uint256).max / 2);
-        addedAmount_ = bound(addedAmount_, 0, type(uint256).max / 2);
-
-        strategy.approve(account_, initialAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-
-        assertTrue(strategy.increaseAllowance(account_, addedAmount_));
-
-        assertEq(
-            strategy.allowance(self, account_),
-            initialAmount_ + addedAmount_
-        );
-    }
-
-    function testFuzz_increaseAllowance_overflows(
-        address account_,
-        uint256 initialAmount_,
-        uint256 addedAmount_
-    ) public {
-        vm.assume(account_ != address(0) && account_ != address(strategy));
-        initialAmount_ = bound(
-            initialAmount_,
-            type(uint256).max / 2 + 1,
-            type(uint256).max
-        );
-        addedAmount_ = bound(
-            addedAmount_,
-            type(uint256).max / 2 + 1,
-            type(uint256).max
-        );
-
-        strategy.approve(account_, initialAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-
-        vm.expectRevert(ARITHMETIC_ERROR);
-        strategy.increaseAllowance(account_, addedAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-    }
-
-    function testFuzz_decreaseAllowance_nonInfiniteApproval(
-        address account_,
-        uint256 initialAmount_,
-        uint256 subtractedAmount_
-    ) public {
-        vm.assume(account_ != address(0) && account_ != address(strategy));
-        initialAmount_ = bound(initialAmount_, 0, type(uint256).max - 1);
-        subtractedAmount_ = bound(subtractedAmount_, 0, initialAmount_);
-
-        strategy.approve(account_, initialAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-
-        assertTrue(strategy.decreaseAllowance(account_, subtractedAmount_));
-
-        assertEq(
-            strategy.allowance(self, account_),
-            initialAmount_ - subtractedAmount_
-        );
-    }
-
-    function testFuzz_decreaseAllowance_underFlows(
-        address account_,
-        uint256 initialAmount_,
-        uint256 subtractedAmount_
-    ) public {
-        vm.assume(account_ != address(0) && account_ != address(strategy));
-        initialAmount_ = bound(initialAmount_, 0, type(uint256).max - 1);
-        subtractedAmount_ = bound(
-            subtractedAmount_,
-            initialAmount_ + 1,
-            type(uint256).max
-        );
-
-        strategy.approve(account_, initialAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-
-        vm.expectRevert(ARITHMETIC_ERROR);
-        strategy.decreaseAllowance(account_, subtractedAmount_);
-
-        assertEq(strategy.allowance(self, account_), initialAmount_);
-    }
-
     function testFuzz_transfer(address account_, uint256 amount_) public {
         vm.assume(account_ != address(0) && account_ != address(strategy));
         amount_ = bound(amount_, minFuzzAmount, maxFuzzAmount);

diff --git a/src/test/handlers/StrategyHandler.sol b/src/test/handlers/StrategyHandler.sol
@@ -193,12 +193,14 @@ contract StrategyHandler is ExtendedTest {
 
         amount = bound(amount, 0, strategy.balanceOf(from));
         uint256 allowance = strategy.allowance(actor, from);
-        if (allowance == 0) {
+        if (allowance != 0) {
             vm.prank(from);
-            strategy.approve(actor, amount);
-        } else if (allowance < amount) {
-            strategy.increaseAllowance(actor, amount - allowance);
+            strategy.approve(actor, 0);
         }
+
+        vm.prank(from);
+        strategy.approve(actor, amount);
+
         if (amount == 0) ghost_zeroTransferFroms++;
 
         vm.prank(actor);