feat: Implement infinite approval (SC-5372) (#38)

* feat: implement infinite approval * chore: fix PR comments * chore: erc20 changes * fix: formatting, add more assertions Co-authored-by: Lucas Manuel <[email protected]>
maple-labs · Mar 28, 2022 · 4d1d6a8 · 4d1d6a8
1 parent d6256a6
commit 4d1d6a8
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 5 deletions.
diff --git a/contracts/ERC20.sol b/contracts/ERC20.sol
@@ -63,7 +63,7 @@ contract ERC20 is IERC20 {
     }
 
     function decreaseAllowance(address spender_, uint256 subtractedAmount_) external override returns (bool success_) {
-        _approve(msg.sender, spender_, allowance[msg.sender][spender_] - subtractedAmount_);
+        _decreaseAllowance(msg.sender, spender_, subtractedAmount_);
         return true;
     }
 
@@ -107,7 +107,7 @@ contract ERC20 is IERC20 {
     }
 
     function transferFrom(address owner_, address recipient_, uint256 amount_) external override returns (bool success_) {
-        _approve(owner_, msg.sender, allowance[owner_][msg.sender] - amount_);
+        _decreaseAllowance(owner_, msg.sender, amount_);
         _transfer(owner_, recipient_, amount_);
         return true;
     }
@@ -145,6 +145,14 @@ contract ERC20 is IERC20 {
         emit Transfer(owner_, address(0), amount_);
     }
 
+    function _decreaseAllowance(address owner_, address spender_, uint256 subtractedAmount_) internal {
+        uint256 spenderAllowance = allowance[owner_][spender_];  // Cache to memory.
+
+        if (spenderAllowance != type(uint256).max) {
+            _approve(owner_, spender_, spenderAllowance - subtractedAmount_);
+        }
+    }
+
     function _mint(address recipient_, uint256 amount_) internal {
         totalSupply += amount_;
 

diff --git a/contracts/test/ERC20.t.sol b/contracts/test/ERC20.t.sol
@@ -72,8 +72,22 @@ contract ERC20BaseTest is TestUtils {
         assertEq(_token.allowance(self, account_), initialAmount_ + addedAmount_);
     }
 
-    function testFuzz_decreaseAllowance(address account_, uint256 initialAmount_, uint256 subtractedAmount_) public {
-        initialAmount_    = constrictToRange(initialAmount_,    0, type(uint256).max);
+    function testFuzz_decreaseAllowance_infiniteApproval(address account_, uint256 subtractedAmount_) public {
+        uint256 MAX_UINT256 = type(uint256).max;
+
+        subtractedAmount_ = constrictToRange(subtractedAmount_, 0, MAX_UINT256);
+
+        _token.approve(account_, MAX_UINT256);
+
+        assertEq(_token.allowance(self, account_), MAX_UINT256);
+
+        assertTrue(_token.decreaseAllowance(account_, subtractedAmount_));
+
+        assertEq(_token.allowance(self, account_), MAX_UINT256);
+    }
+
+    function testFuzz_decreaseAllowance_nonInfiniteApproval(address account_, uint256 initialAmount_, uint256 subtractedAmount_) public {
+        initialAmount_    = constrictToRange(initialAmount_,    0, type(uint256).max - 1);
         subtractedAmount_ = constrictToRange(subtractedAmount_, 0, initialAmount_);
 
         _token.approve(account_, initialAmount_);
@@ -101,7 +115,8 @@ contract ERC20BaseTest is TestUtils {
     }
 
     function testFuzz_transferFrom(address recipient_, uint256 approval_, uint256 amount_) public {
-        if (amount_ > approval_) return;  // Owner must approve for more than amount.
+        approval_ = constrictToRange(approval_, 0, type(uint256).max - 1);
+        amount_   = constrictToRange(amount_,   0, approval_);
 
         ERC20User owner = new ERC20User();
 
@@ -124,6 +139,33 @@ contract ERC20BaseTest is TestUtils {
         }
     }
 
+    function testFuzz_transferFrom_infiniteApproval(address recipient_, uint256 amount_) public {
+        uint256 MAX_UINT256 = type(uint256).max;
+
+        amount_ = constrictToRange(amount_, 0, MAX_UINT256);
+
+        ERC20User owner = new ERC20User();
+
+        _token.mint(address(owner), amount_);
+        owner.erc20_approve(address(_token), self, MAX_UINT256);
+
+        assertEq(_token.balanceOf(address(owner)),       amount_);
+        assertEq(_token.totalSupply(),                   amount_);
+        assertEq(_token.allowance(address(owner), self), MAX_UINT256);
+
+        assertTrue(_token.transferFrom(address(owner), recipient_, amount_));
+
+        assertEq(_token.totalSupply(),                   amount_);
+        assertEq(_token.allowance(address(owner), self), MAX_UINT256);
+
+        if (address(owner) == recipient_) {
+            assertEq(_token.balanceOf(address(owner)), amount_);
+        } else {
+            assertEq(_token.balanceOf(address(owner)), 0);
+            assertEq(_token.balanceOf(recipient_),     amount_);
+        }
+    }
+
     function testFuzz_transfer_insufficientBalance(address recipient_, uint256 amount_) public {
         amount_ = amount_ == 0 ? 1 : amount_;