Skip to content

Commit

Permalink
Add a _spendAllowance function to ERC20 & ERC777 (#3170)
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx authored Feb 9, 2022
1 parent 63b4669 commit c5a6cae
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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))
Expand Down
32 changes: 23 additions & 9 deletions contracts/token/ERC20/ERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand Down
6 changes: 1 addition & 5 deletions contracts/token/ERC20/extensions/ERC20Burnable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
32 changes: 23 additions & 9 deletions contracts/token/ERC777/ERC777.sol
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,8 @@ contract ERC777 is Context, IERC777, IERC20 {
uint256 amount
) public virtual override returns (bool) {
address spender = _msgSender();
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);
_send(holder, recipient, amount, "", "", false);

return true;
}

Expand Down Expand Up @@ -509,6 +501,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.
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/ERC20.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/extensions/ERC20Burnable.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/token/ERC20/extensions/ERC20Wrapper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
});

Expand Down

0 comments on commit c5a6cae

Please sign in to comment.