diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 7c53c696254..5293ed76af3 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -1,12 +1,42 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v4.8.0) (token/ERC20/ERC20.sol) -pragma solidity ^0.8.0; +pragma solidity ^0.8.4; import "./IERC20.sol"; import "./extensions/IERC20Metadata.sol"; import "../../utils/Context.sol"; +/** + * @dev Implementation of solidity 0.8.4 custom errors. + * + * This implementation adopted the convention of errors before and outside the contract + * code. + * + * The following are EIP-6093 implementations: + * https://eips.ethereum.org/EIPS/eip-6093 + */ + +// Indicates an error related to the current balance of a sender. Used in transfers. +error ERC20InsufficientBalance(address sender, uint256 balance, uint256 needed); +// Indicates a failure with the token sender. Used in transfers. +error ERC20InvalidSender(address sender); +// Indicates a failure with the token receiver. Used in transfers. +error ERC20InvalidReceiver(address receiver); +// Indicates a failure with the spender’s allowance. Used in transfers. +error ERC20InsufficientAllowance(address spender, uint256 allowance, uint256 needed); +// Indicates a failure with the approver of a token to be approved. Used in approvals. +error ERC20InvalidApprover(address approver); +// Indicates a failure with the spender to be approved. Used in approvals. +error ERC20InvalidSpender(address spender); + +/** The following are custom implementations not present at EIP-6093 based on + * legacy requires. + */ + +// Indicates an error related to reducing the allowance to less than zero +error ERC20InvalidAllowanceReduction(uint256 allowance, uint256 reduction); + /** * @dev Implementation of the {IERC20} interface. * @@ -197,7 +227,15 @@ contract ERC20 is Context, IERC20, IERC20Metadata { function decreaseAllowance(address spender, uint256 subtractedValue) public virtual returns (bool) { address owner = _msgSender(); uint256 currentAllowance = allowance(owner, spender); - require(currentAllowance >= subtractedValue, "ERC20: decreased allowance below zero"); + + if (spender == address(0)) { + revert ERC20InvalidSpender(address(0)); + } + + if (currentAllowance < subtractedValue) { + revert ERC20InvalidAllowanceReduction(currentAllowance, subtractedValue); + } + unchecked { _approve(owner, spender, currentAllowance - subtractedValue); } @@ -220,13 +258,22 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - `from` must have a balance of at least `amount`. */ function _transfer(address from, address to, uint256 amount) internal virtual { - require(from != address(0), "ERC20: transfer from the zero address"); - require(to != address(0), "ERC20: transfer to the zero address"); + if (from == address(0)) { + revert ERC20InvalidSender(from); + } + + if (to == address(0)) { + revert ERC20InvalidReceiver(to); + } _beforeTokenTransfer(from, to, amount); uint256 fromBalance = _balances[from]; - require(fromBalance >= amount, "ERC20: transfer amount exceeds balance"); + + if (fromBalance < amount) { + revert ERC20InsufficientBalance(from, fromBalance, amount); + } + unchecked { _balances[from] = fromBalance - amount; // Overflow not possible: the sum of all balances is capped by totalSupply, and the sum is preserved by @@ -249,7 +296,9 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - `account` cannot be the zero address. */ function _mint(address account, uint256 amount) internal virtual { - require(account != address(0), "ERC20: mint to the zero address"); + if (account == address(0)) { + revert ERC20InvalidReceiver(account); + } _beforeTokenTransfer(address(0), account, amount); @@ -275,12 +324,18 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - `account` must have at least `amount` tokens. */ function _burn(address account, uint256 amount) internal virtual { - require(account != address(0), "ERC20: burn from the zero address"); + if (account == address(0)) { + revert ERC20InvalidSender(account); + } _beforeTokenTransfer(account, address(0), amount); uint256 accountBalance = _balances[account]; - require(accountBalance >= amount, "ERC20: burn amount exceeds balance"); + + if (accountBalance < amount) { + revert ERC20InsufficientBalance(account, accountBalance, amount); + } + unchecked { _balances[account] = accountBalance - amount; // Overflow not possible: amount <= accountBalance <= totalSupply. @@ -306,8 +361,13 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - `spender` cannot be the zero address. */ function _approve(address owner, address spender, uint256 amount) internal virtual { - require(owner != address(0), "ERC20: approve from the zero address"); - require(spender != address(0), "ERC20: approve to the zero address"); + if (owner == address(0)) { + revert ERC20InvalidApprover(owner); + } + + if (spender == address(0)) { + revert ERC20InvalidSpender(spender); + } _allowances[owner][spender] = amount; emit Approval(owner, spender, amount); @@ -324,7 +384,10 @@ contract ERC20 is Context, IERC20, IERC20Metadata { 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"); + if (currentAllowance < amount) { + revert ERC20InsufficientAllowance(spender, currentAllowance, amount); + } + unchecked { _approve(owner, spender, currentAllowance - amount); } diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 41e47f06528..b5cd909ca24 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -2,6 +2,8 @@ const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test const { expect } = require('chai'); const { ZERO_ADDRESS, MAX_UINT256 } = constants; +const CUSTOM_ERROR = 'reverted with custom error '; + function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipient, anotherAccount) { describe('total supply', function () { it('returns the total amount of tokens', async function () { @@ -87,7 +89,7 @@ function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipi it('reverts', async function () { await expectRevert( this.token.transferFrom(tokenOwner, to, amount, { from: spender }), - `${errorPrefix}: transfer amount exceeds balance`, + CUSTOM_ERROR + `'${errorPrefix}InsufficientBalance("${tokenOwner}", ${initialSupply - 1}, ${amount})'`, ); }); }); @@ -106,7 +108,7 @@ function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipi it('reverts', async function () { await expectRevert( this.token.transferFrom(tokenOwner, to, amount, { from: spender }), - `${errorPrefix}: insufficient allowance`, + CUSTOM_ERROR + `'${errorPrefix}InsufficientAllowance("${spender}", ${initialSupply - 1}, ${amount})'`, ); }); }); @@ -121,7 +123,7 @@ function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipi it('reverts', async function () { await expectRevert( this.token.transferFrom(tokenOwner, to, amount, { from: spender }), - `${errorPrefix}: transfer amount exceeds balance`, + CUSTOM_ERROR + `'${errorPrefix}InsufficientBalance("${tokenOwner}", ${initialSupply - 2}, ${amount})'`, ); }); }); @@ -155,7 +157,7 @@ function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipi it('reverts', async function () { await expectRevert( this.token.transferFrom(tokenOwner, to, amount, { from: spender }), - `${errorPrefix}: transfer to the zero address`, + CUSTOM_ERROR + `'${errorPrefix}InvalidReceiver("${to}")'`, ); }); }); @@ -167,7 +169,10 @@ function shouldBehaveLikeERC20(errorPrefix, initialSupply, initialHolder, recipi const to = recipient; it('reverts', async function () { - await expectRevert(this.token.transferFrom(tokenOwner, to, amount, { from: spender }), 'from the zero address'); + await expectRevert( + this.token.transferFrom(tokenOwner, to, amount, { from: spender }), + CUSTOM_ERROR + `'${errorPrefix}InvalidApprover("${tokenOwner}")'`, + ); }); }); }); @@ -191,7 +196,10 @@ function shouldBehaveLikeERC20Transfer(errorPrefix, from, to, balance, transfer) const amount = balance.addn(1); it('reverts', async function () { - await expectRevert(transfer.call(this, from, to, amount), `${errorPrefix}: transfer amount exceeds balance`); + await expectRevert( + transfer.call(this, from, to, amount), + CUSTOM_ERROR + `'${errorPrefix}InsufficientBalance("${from}", ${balance}, ${amount})'`, + ); }); }); @@ -232,7 +240,7 @@ function shouldBehaveLikeERC20Transfer(errorPrefix, from, to, balance, transfer) it('reverts', async function () { await expectRevert( transfer.call(this, from, ZERO_ADDRESS, balance), - `${errorPrefix}: transfer to the zero address`, + CUSTOM_ERROR + `'${errorPrefix}InvalidReceiver("${ZERO_ADDRESS}")'`, ); }); }); @@ -309,7 +317,7 @@ function shouldBehaveLikeERC20Approve(errorPrefix, owner, spender, supply, appro it('reverts', async function () { await expectRevert( approve.call(this, owner, ZERO_ADDRESS, supply), - `${errorPrefix}: approve to the zero address`, + CUSTOM_ERROR + `'${errorPrefix}InvalidSpender("${ZERO_ADDRESS}")'`, ); }); }); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 6971eede981..46851c0029f 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -11,6 +11,8 @@ const { const ERC20 = artifacts.require('$ERC20'); const ERC20Decimals = artifacts.require('$ERC20DecimalsMock'); +const CUSTOM_ERROR = 'reverted with custom error '; + contract('ERC20', function (accounts) { const [initialHolder, recipient, anotherAccount] = accounts; @@ -56,7 +58,7 @@ contract('ERC20', function (accounts) { it('reverts', async function () { await expectRevert( this.token.decreaseAllowance(spender, amount, { from: initialHolder }), - 'ERC20: decreased allowance below zero', + CUSTOM_ERROR + `'ERC20InvalidAllowanceReduction(0, ${amount})'`, ); }); }); @@ -90,7 +92,7 @@ contract('ERC20', function (accounts) { it('reverts when more than the full allowance is removed', async function () { await expectRevert( this.token.decreaseAllowance(spender, approvedAmount.addn(1), { from: initialHolder }), - 'ERC20: decreased allowance below zero', + CUSTOM_ERROR + `'ERC20InvalidAllowanceReduction(${approvedAmount}, ${approvedAmount.addn(1)})'`, ); }); }); @@ -116,7 +118,7 @@ contract('ERC20', function (accounts) { it('reverts', async function () { await expectRevert( this.token.decreaseAllowance(spender, amount, { from: initialHolder }), - 'ERC20: decreased allowance below zero', + CUSTOM_ERROR + `'ERC20InvalidSpender("${spender}")'`, ); }); }); @@ -197,7 +199,7 @@ contract('ERC20', function (accounts) { it('reverts', async function () { await expectRevert( this.token.increaseAllowance(spender, amount, { from: initialHolder }), - 'ERC20: approve to the zero address', + CUSTOM_ERROR + `'ERC20InvalidSpender("${spender}")'`, ); }); }); @@ -206,7 +208,10 @@ contract('ERC20', function (accounts) { describe('_mint', function () { const amount = new BN(50); it('rejects a null account', async function () { - await expectRevert(this.token.$_mint(ZERO_ADDRESS, amount), 'ERC20: mint to the zero address'); + await expectRevert( + this.token.$_mint(ZERO_ADDRESS, amount), + CUSTOM_ERROR + `'ERC20InvalidReceiver("${ZERO_ADDRESS}")'`, + ); }); describe('for a non zero account', function () { @@ -233,14 +238,17 @@ contract('ERC20', function (accounts) { describe('_burn', function () { it('rejects a null account', async function () { - await expectRevert(this.token.$_burn(ZERO_ADDRESS, new BN(1)), 'ERC20: burn from the zero address'); + await expectRevert( + this.token.$_burn(ZERO_ADDRESS, new BN(1)), + CUSTOM_ERROR + `'ERC20InvalidSender("${ZERO_ADDRESS}")'`, + ); }); describe('for a non zero account', function () { it('rejects burning more than balance', async function () { await expectRevert( this.token.$_burn(initialHolder, initialSupply.addn(1)), - 'ERC20: burn amount exceeds balance', + CUSTOM_ERROR + `'ERC20InsufficientBalance("${initialHolder}", ${initialSupply}, ${initialSupply.addn(1)})'`, ); }); @@ -282,7 +290,7 @@ contract('ERC20', function (accounts) { it('reverts', async function () { await expectRevert( this.token.$_transfer(ZERO_ADDRESS, recipient, initialSupply), - 'ERC20: transfer from the zero address', + CUSTOM_ERROR + `'ERC20InvalidSender("${ZERO_ADDRESS}")'`, ); }); }); @@ -297,7 +305,7 @@ contract('ERC20', function (accounts) { it('reverts', async function () { await expectRevert( this.token.$_approve(ZERO_ADDRESS, recipient, initialSupply), - 'ERC20: approve from the zero address', + CUSTOM_ERROR + `'ERC20InvalidApprover("${ZERO_ADDRESS}")'`, ); }); }); diff --git a/test/token/ERC20/extensions/ERC20Burnable.behavior.js b/test/token/ERC20/extensions/ERC20Burnable.behavior.js index 2edabc4bfd9..97d7a7eb4c7 100644 --- a/test/token/ERC20/extensions/ERC20Burnable.behavior.js +++ b/test/token/ERC20/extensions/ERC20Burnable.behavior.js @@ -3,6 +3,8 @@ const { ZERO_ADDRESS } = constants; const { expect } = require('chai'); +const CUSTOM_ERROR = 'reverted with custom error '; + function shouldBehaveLikeERC20Burnable(owner, initialBalance, [burner]) { describe('burn', function () { describe('when the given amount is not greater than balance of the sender', function () { @@ -37,7 +39,10 @@ function shouldBehaveLikeERC20Burnable(owner, initialBalance, [burner]) { const amount = initialBalance.addn(1); it('reverts', async function () { - await expectRevert(this.token.burn(amount, { from: owner }), 'ERC20: burn amount exceeds balance'); + await expectRevert( + this.token.burn(amount, { from: owner }), + CUSTOM_ERROR + `'ERC20InsufficientBalance("${owner}", ${initialBalance}, ${amount})'`, + ); }); }); }); @@ -83,7 +88,10 @@ function shouldBehaveLikeERC20Burnable(owner, initialBalance, [burner]) { it('reverts', async function () { await this.token.approve(burner, amount, { from: owner }); - await expectRevert(this.token.burnFrom(owner, amount, { from: burner }), 'ERC20: burn amount exceeds balance'); + await expectRevert( + this.token.burnFrom(owner, amount, { from: burner }), + CUSTOM_ERROR + `'ERC20InsufficientBalance("${owner}", ${initialBalance}, ${amount})'`, + ); }); }); @@ -94,7 +102,7 @@ function shouldBehaveLikeERC20Burnable(owner, initialBalance, [burner]) { await this.token.approve(burner, allowance, { from: owner }); await expectRevert( this.token.burnFrom(owner, allowance.addn(1), { from: burner }), - 'ERC20: insufficient allowance', + CUSTOM_ERROR + `'ERC20InsufficientAllowance("${burner}", ${allowance}, ${allowance.addn(1)})'`, ); }); }); diff --git a/test/token/ERC20/extensions/ERC20FlashMint.test.js b/test/token/ERC20/extensions/ERC20FlashMint.test.js index 76d66ff7a37..6a78791b2b2 100644 --- a/test/token/ERC20/extensions/ERC20FlashMint.test.js +++ b/test/token/ERC20/extensions/ERC20FlashMint.test.js @@ -7,6 +7,8 @@ const { MAX_UINT256, ZERO_ADDRESS } = constants; const ERC20FlashMintMock = artifacts.require('$ERC20FlashMintMock'); const ERC3156FlashBorrowerMock = artifacts.require('ERC3156FlashBorrowerMock'); +const CUSTOM_ERROR = 'reverted with custom error '; + contract('ERC20FlashMint', function (accounts) { const [initialHolder, other, anotherAccount] = accounts; @@ -89,7 +91,7 @@ contract('ERC20FlashMint', function (accounts) { const receiver = await ERC3156FlashBorrowerMock.new(true, false); await expectRevert( this.token.flashLoan(receiver.address, this.token.address, loanAmount, '0x'), - 'ERC20: insufficient allowance', + CUSTOM_ERROR + `'ERC20InsufficientAllowance("${this.token.address}", 0, ${loanAmount})'`, ); }); @@ -98,7 +100,7 @@ contract('ERC20FlashMint', function (accounts) { const data = this.token.contract.methods.transfer(other, 10).encodeABI(); await expectRevert( this.token.flashLoan(receiver.address, this.token.address, loanAmount, data), - 'ERC20: burn amount exceeds balance', + CUSTOM_ERROR + `'ERC20InsufficientBalance("${receiver.address}", ${loanAmount.addn(-10)}, ${loanAmount})'`, ); }); diff --git a/test/token/ERC20/extensions/ERC20Wrapper.test.js b/test/token/ERC20/extensions/ERC20Wrapper.test.js index cfb47bfa289..819144663e1 100644 --- a/test/token/ERC20/extensions/ERC20Wrapper.test.js +++ b/test/token/ERC20/extensions/ERC20Wrapper.test.js @@ -8,6 +8,8 @@ const NotAnERC20 = artifacts.require('CallReceiverMock'); const ERC20Decimals = artifacts.require('$ERC20DecimalsMock'); const ERC20Wrapper = artifacts.require('$ERC20Wrapper'); +const CUSTOM_ERROR = 'reverted with custom error '; + contract('ERC20', function (accounts) { const [initialHolder, recipient, anotherAccount] = accounts; @@ -68,7 +70,7 @@ contract('ERC20', function (accounts) { it('missing approval', async function () { await expectRevert( this.token.depositFor(initialHolder, initialSupply, { from: initialHolder }), - 'ERC20: insufficient allowance', + CUSTOM_ERROR + `'ERC20InsufficientAllowance("${this.token.address}", 0, ${initialSupply})'`, ); }); @@ -76,7 +78,7 @@ contract('ERC20', function (accounts) { await this.underlying.approve(this.token.address, MAX_UINT256, { from: initialHolder }); await expectRevert( this.token.depositFor(initialHolder, MAX_UINT256, { from: initialHolder }), - 'ERC20: transfer amount exceeds balance', + CUSTOM_ERROR + `'ERC20InsufficientBalance("${initialHolder}", ${initialSupply}, ${MAX_UINT256})'`, ); }); @@ -105,7 +107,7 @@ contract('ERC20', function (accounts) { it('missing balance', async function () { await expectRevert( this.token.withdrawTo(initialHolder, MAX_UINT256, { from: initialHolder }), - 'ERC20: burn amount exceeds balance', + CUSTOM_ERROR + `'ERC20InsufficientBalance("${initialHolder}", ${initialSupply}, ${MAX_UINT256})'`, ); });