diff --git a/CHANGELOG.md b/CHANGELOG.md index 784fb92b98d..07134ddf2a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,10 @@ * `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)) + * `ERC20`: add a `_spendAllowance` internal function. ([#3170](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3170)) + * `ERC20Burnable`: do not update allowance on `burnFrom` when allowance is `type(uint256).max`. ([#3170](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3170)) * `ERC777`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3085)) + * `ERC777`: add a `_spendAllowance` internal function. ([#3170](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3170)) * `SignedMath`: a new signed version of the Math library with `max`, `min`, and `average`. ([#2686](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2686)) * `SignedMath`: add a `abs(int256)` method that returns the unsigned absolute value of a signed value. ([#2984](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2984)) * `ERC1967Upgrade`: Refactor the secure upgrade to use `ERC1822` instead of the previous rollback mechanism. This reduces code complexity and attack surface with similar security guarantees. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index c65b69ecd42..a888a7ce6bf 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -161,16 +161,8 @@ contract ERC20 is Context, IERC20, IERC20Metadata { uint256 amount ) public virtual override returns (bool) { address spender = _msgSender(); - uint256 currentAllowance = allowance(from, spender); - if (currentAllowance != type(uint256).max) { - require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance"); - unchecked { - _approve(from, spender, currentAllowance - amount); - } - } - + _spendAllowance(from, spender, amount); _transfer(from, to, amount); - return true; } @@ -327,6 +319,28 @@ contract ERC20 is Context, IERC20, IERC20Metadata { emit Approval(owner, spender, amount); } + /** + * @dev Spend `amount` form the allowance of `owner` toward `spender`. + * + * Does not update the allowance amount in case of infinite allowance. + * Revert if not enough allowance is available. + * + * Might emit an {Approval} event. + */ + function _spendAllowance( + address owner, + address spender, + uint256 amount + ) internal virtual { + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC20: insufficient allowance"); + unchecked { + _approve(owner, spender, currentAllowance - amount); + } + } + } + /** * @dev Hook that is called before any transfer of tokens. This includes * minting and burning. diff --git a/contracts/token/ERC20/extensions/ERC20Burnable.sol b/contracts/token/ERC20/extensions/ERC20Burnable.sol index ab961a9e26b..8a7c11311b9 100644 --- a/contracts/token/ERC20/extensions/ERC20Burnable.sol +++ b/contracts/token/ERC20/extensions/ERC20Burnable.sol @@ -33,11 +33,7 @@ abstract contract ERC20Burnable is Context, ERC20 { * `amount`. */ function burnFrom(address account, uint256 amount) public virtual { - uint256 currentAllowance = allowance(account, _msgSender()); - require(currentAllowance >= amount, "ERC20: burn amount exceeds allowance"); - unchecked { - _approve(account, _msgSender(), currentAllowance - amount); - } + _spendAllowance(account, _msgSender(), amount); _burn(account, amount); } } diff --git a/contracts/token/ERC777/ERC777.sol b/contracts/token/ERC777/ERC777.sol index 534f4bb1d54..a1df2c4c7de 100644 --- a/contracts/token/ERC777/ERC777.sol +++ b/contracts/token/ERC777/ERC777.sol @@ -293,13 +293,7 @@ contract ERC777 is Context, IERC777, IERC20 { _callTokensToSend(spender, holder, recipient, amount, "", ""); - uint256 currentAllowance = _allowances[holder][spender]; - if (currentAllowance != type(uint256).max) { - require(currentAllowance >= amount, "ERC777: transfer amount exceeds allowance"); - unchecked { - _approve(holder, spender, currentAllowance - amount); - } - } + _spendAllowance(holder, spender, amount); _move(spender, holder, recipient, amount, "", ""); @@ -526,6 +520,28 @@ contract ERC777 is Context, IERC777, IERC20 { } } + /** + * @dev Spend `amount` form the allowance of `owner` toward `spender`. + * + * Does not update the allowance amount in case of infinite allowance. + * Revert if not enough allowance is available. + * + * Might emit an {Approval} event. + */ + function _spendAllowance( + address owner, + address spender, + uint256 amount + ) internal virtual { + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC777: insufficient allowance"); + unchecked { + _approve(owner, spender, currentAllowance - amount); + } + } + } + /** * @dev Hook that is called before any token transfer. This includes * calls to {send}, {transfer}, {operatorSend}, minting and burning. diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 3489ab05390..8bc54762ac8 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -108,7 +108,7 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip it('reverts', async function () { await expectRevert( this.token.transferFrom(tokenOwner, to, amount, { from: spender }), - `${errorPrefix}: transfer amount exceeds allowance`, + `${errorPrefix}: insufficient allowance`, ); }); }); diff --git a/test/token/ERC20/extensions/ERC20Burnable.behavior.js b/test/token/ERC20/extensions/ERC20Burnable.behavior.js index 0fa2f11cda5..3ba2fc9d91f 100644 --- a/test/token/ERC20/extensions/ERC20Burnable.behavior.js +++ b/test/token/ERC20/extensions/ERC20Burnable.behavior.js @@ -98,7 +98,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { it('reverts', async function () { await this.token.approve(burner, allowance, { from: owner }); await expectRevert(this.token.burnFrom(owner, allowance.addn(1), { from: burner }), - 'ERC20: burn amount exceeds allowance', + 'ERC20: insufficient allowance', ); }); }); diff --git a/test/token/ERC20/extensions/ERC20Wrapper.test.js b/test/token/ERC20/extensions/ERC20Wrapper.test.js index 05652342a80..ec074e1b8cf 100644 --- a/test/token/ERC20/extensions/ERC20Wrapper.test.js +++ b/test/token/ERC20/extensions/ERC20Wrapper.test.js @@ -59,7 +59,7 @@ contract('ERC20', function (accounts) { it('missing approval', async function () { await expectRevert( this.token.depositFor(initialHolder, initialSupply, { from: initialHolder }), - 'ERC20: transfer amount exceeds allowance', + 'ERC20: insufficient allowance', ); });