Skip to content

Commit

Permalink
Do not emit Approval event when calling transferFrom (OpenZeppelin#4370)
Browse files Browse the repository at this point in the history
Co-authored-by: Ernesto García <ernestognw@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
  • Loading branch information
3 people committed Jul 3, 2023
1 parent f6cc3af commit abbbaef
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 80 deletions.
2 changes: 1 addition & 1 deletion contracts/mocks/token/ERC20ApprovalMock.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {ERC20} from "../../token/ERC20/ERC20.sol";
import "../../token/ERC20/ERC20.sol";

abstract contract ERC20ApprovalMock is ERC20 {
function _approve(address owner, address spender, uint256 amount, bool) internal virtual override {
Expand Down
16 changes: 8 additions & 8 deletions contracts/token/ERC20/ERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,8 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
* - `owner` cannot be the zero address.
* - `spender` cannot be the zero address.
*/
function _approve(address owner, address spender, uint256 value) internal virtual {
_approve(owner, spender, value, true);
function _approve(address owner, address spender, uint256 amount) internal virtual {
_approve(owner, spender, amount, true);
}

/**
Expand All @@ -328,23 +328,23 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
* Anyone who wishes to continue emitting `Approval` events on the`transferFrom` operation can force the flag to true
* using the following override:
* ```
* function _approve(address owner, address spender, uint256 value, bool) internal virtual override {
* super._approve(owner, spender, value, true);
* function _approve(address owner, address spender, uint256 amount, bool) internal virtual override {
* super._approve(owner, spender, amount, true);
* }
* ```
*
* Requirements are the same as {_approve}.
*/
function _approve(address owner, address spender, uint256 value, bool emitEvent) internal virtual {
function _approve(address owner, address spender, uint256 amount, bool emitEvent) internal virtual {
if (owner == address(0)) {
revert ERC20InvalidApprover(address(0));
}
if (spender == address(0)) {
revert ERC20InvalidSpender(address(0));
}
_allowances[owner][spender] = value;
_allowances[owner][spender] = amount;
if (emitEvent) {
emit Approval(owner, spender, value);
emit Approval(owner, spender, amount);
}
}

Expand All @@ -363,7 +363,7 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors {
revert ERC20InsufficientAllowance(spender, currentAllowance, value);
}
unchecked {
_approve(owner, spender, currentAllowance - value, false);
_approve(owner, spender, currentAllowance - amount, false);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/token/ERC20/ERC20.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function shouldBehaveLikeERC20(initialSupply, accounts, opts = {}) {

if (forcedApproval) {
it('emits an approval event', async function () {
expectEvent(await this.token.transferFrom(tokenOwner, to, value, { from: spender }), 'Approval', {
expectEvent(await this.token.transferFrom(tokenOwner, to, amount, { from: spender }), 'Approval', {
owner: tokenOwner,
spender: spender,
value: await this.token.allowance(tokenOwner, spender),
Expand All @@ -84,7 +84,7 @@ function shouldBehaveLikeERC20(initialSupply, accounts, opts = {}) {
} else {
it('does not emit an approval event', async function () {
expectEvent.notEmitted(
await this.token.transferFrom(tokenOwner, to, value, { from: spender }),
await this.token.transferFrom(tokenOwner, to, amount, { from: spender }),
'Approval',
);
});
Expand Down
Loading

0 comments on commit abbbaef

Please sign in to comment.