diff --git a/contracts/ERC20.sol b/contracts/ERC20.sol index dd15608..1dd6c11 100644 --- 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 index 8e60578..297fca8 100644 --- 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_;