diff --git a/CHANGELOG.md b/CHANGELOG.md index b1600733fdc..2107bef05aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ * `ERC2771Context`: use immutable storage to store the forwarder address, no longer an issue since Solidity >=0.8.8 allows reading immutable variables in the constructor. ([#2917](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2917)) * `Base64`: add a library to parse bytes into base64 strings using `encode(bytes memory)` function, and provide examples to show how to use to build URL-safe `tokenURIs`. ([#2884](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2884)) * `ERC20`: reduce allowance before triggering transfer. ([#3056](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3056)) + * `ERC20`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085)) + * `ERC777`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085)) ### Breaking change diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 054159e3dda..fa30163bad2 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -125,6 +125,9 @@ contract ERC20 is Context, IERC20, IERC20Metadata { /** * @dev See {IERC20-approve}. * + * NOTE: If `amount` is the maximum `uint256`, the allowance is not updated on + * `transferFrom`. This is semantically equivalent to an infinite approval. + * * Requirements: * * - `spender` cannot be the zero address. @@ -140,6 +143,9 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * Emits an {Approval} event indicating the updated allowance. This is not * required by the EIP. See the note at the beginning of {ERC20}. * + * NOTE: Does not update the allowance if the current allowance + * is the maximum `uint256`. + * * Requirements: * * - `sender` and `recipient` cannot be the zero address. @@ -153,9 +159,11 @@ contract ERC20 is Context, IERC20, IERC20Metadata { uint256 amount ) public virtual override returns (bool) { uint256 currentAllowance = _allowances[sender][_msgSender()]; - require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance"); - unchecked { - _approve(sender, _msgSender(), currentAllowance - amount); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance"); + unchecked { + _approve(sender, _msgSender(), currentAllowance - amount); + } } _transfer(sender, recipient, amount); diff --git a/contracts/token/ERC777/ERC777.sol b/contracts/token/ERC777/ERC777.sol index f89e1e0242c..302e3f1722e 100644 --- a/contracts/token/ERC777/ERC777.sol +++ b/contracts/token/ERC777/ERC777.sol @@ -258,6 +258,9 @@ contract ERC777 is Context, IERC777, IERC20 { /** * @dev See {IERC20-approve}. * + * NOTE: If `value` is the maximum `uint256`, the allowance is not updated on + * `transferFrom`. This is semantically equivalent to an infinite approval. + * * Note that accounts cannot have allowance issued by their operators. */ function approve(address spender, uint256 value) public virtual override returns (bool) { @@ -269,6 +272,9 @@ contract ERC777 is Context, IERC777, IERC20 { /** * @dev See {IERC20-transferFrom}. * + * NOTE: Does not update the allowance if the current allowance + * is the maximum `uint256`. + * * Note that operator and allowance concepts are orthogonal: operators cannot * call `transferFrom` (unless they have allowance), and accounts with * allowance cannot call `operatorSend` (unless they are operators). @@ -288,8 +294,12 @@ contract ERC777 is Context, IERC777, IERC20 { _callTokensToSend(spender, holder, recipient, amount, "", ""); uint256 currentAllowance = _allowances[holder][spender]; - require(currentAllowance >= amount, "ERC777: transfer amount exceeds allowance"); - _approve(holder, spender, currentAllowance - amount); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC777: transfer amount exceeds allowance"); + unchecked { + _approve(holder, spender, currentAllowance - amount); + } + } _move(spender, holder, recipient, amount, "", ""); diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 55bcc36084f..3489ab05390 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -1,6 +1,6 @@ const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const { ZERO_ADDRESS } = constants; +const { ZERO_ADDRESS, MAX_UINT256 } = constants; function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recipient, anotherAccount) { describe('total supply', function () { @@ -128,6 +128,25 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip }); }); }); + + describe('when the spender has unlimited allowance', function () { + beforeEach(async function () { + await this.token.approve(spender, MAX_UINT256, { from: initialHolder }); + }); + + it('does not decrease the spender allowance', async function () { + await this.token.transferFrom(tokenOwner, to, 1, { from: spender }); + + expect(await this.token.allowance(tokenOwner, spender)).to.be.bignumber.equal(MAX_UINT256); + }); + + it('does not emit an approval event', async function () { + expectEvent.notEmitted( + await this.token.transferFrom(tokenOwner, to, 1, { from: spender }), + 'Approval', + ); + }); + }); }); describe('when the recipient is the zero address', function () {