From bbf64d32291590dba52c6b5806eb274794be8f65 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 16:47:50 +0200 Subject: [PATCH 01/23] Add `ERC20TemporaryApproval` --- .changeset/serious-carrots-provide.md | 5 ++ contracts/interfaces/README.adoc | 3 + contracts/interfaces/draft-IERC7674.sol | 7 ++ contracts/token/ERC20/README.adoc | 5 +- .../draft-ERC20TemporaryApproval.sol | 87 +++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 .changeset/serious-carrots-provide.md create mode 100644 contracts/interfaces/draft-IERC7674.sol create mode 100644 contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md new file mode 100644 index 00000000000..59e58a7ed05 --- /dev/null +++ b/.changeset/serious-carrots-provide.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC20TemporaryAllowance`: add an ERC-20 extension that implements temporary approval using transient storage diff --git a/contracts/interfaces/README.adoc b/contracts/interfaces/README.adoc index 379a24a1e26..61aae05d167 100644 --- a/contracts/interfaces/README.adoc +++ b/contracts/interfaces/README.adoc @@ -40,6 +40,7 @@ are useful to interact with third party contracts that implement them. - {IERC5313} - {IERC5805} - {IERC6372} +- {IERC7674} == Detailed ABI @@ -80,3 +81,5 @@ are useful to interact with third party contracts that implement them. {{IERC5805}} {{IERC6372}} + +{{IERC7674}} diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol new file mode 100644 index 00000000000..3be8bec2f8b --- /dev/null +++ b/contracts/interfaces/draft-IERC7674.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +interface IERC7674 { + function temporaryApprove(address spender, uint256 value) external returns (bool success); +} diff --git a/contracts/token/ERC20/README.adoc b/contracts/token/ERC20/README.adoc index 938784ff996..7a5d783d299 100644 --- a/contracts/token/ERC20/README.adoc +++ b/contracts/token/ERC20/README.adoc @@ -22,6 +22,7 @@ Additionally there are multiple custom extensions, including: * {ERC20FlashMint}: token level support for flash loans through the minting and burning of ephemeral tokens (standardized as ERC-3156). * {ERC20Votes}: support for voting and vote delegation. * {ERC20Wrapper}: wrapper to create an ERC-20 backed by another ERC-20, with deposit and withdraw methods. Useful in conjunction with {ERC20Votes}. +* {ERC20TemporaryApproval}: support for temporary allowances following ERC-7674. * {ERC1363}: support for calling the target of a transfer or approval, enabling code execution on the receiver within a single transaction. * {ERC4626}: tokenized vault that manages shares (represented as ERC-20) that are backed by assets (another ERC-20). @@ -55,11 +56,13 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC20Pausable}} +{{ERC20FlashMint}} + {{ERC20Votes}} {{ERC20Wrapper}} -{{ERC20FlashMint}} +{{ERC20TemporaryApproval}} {{ERC1363}} diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol new file mode 100644 index 00000000000..515e1150222 --- /dev/null +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {ERC20} from "../ERC20.sol"; +import {IERC7674} from "../../../interfaces/draft-IERC7674.sol"; +import {Math} from "../../../utils/math/Math.sol"; +import {SlotDerivation} from "../../../utils/SlotDerivation.sol"; +import {StorageSlot} from "../../../utils/StorageSlot.sol"; + +/** + * @dev Extension of {ERC20} that adds support for temporary allowances following ERC-7674. + * + * WARNING: This is a draft contract. The corresponding ERC is still subject to changes. + */ +abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { + using SlotDerivation for bytes32; + using StorageSlot for bytes32; + using StorageSlot for StorageSlot.Uint256SlotType; + + // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20TemporaryApproval")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant ERC20TemporaryApprovalStorageLocation = + 0x0fd66af0be6cb88466bb5c49c7ea8fbb4acdc82057e863d0a17fddeaaf18fe00; + + /** + * @dev {allowance} override that includes the temporary allowance when looking up the current allowance. If + * adding up the "traditional" and the temporary allowances result in an overflow, type(uint256).max is returned. + */ + function allowance(address owner, address spender) public view virtual override returns (uint256) { + (bool success, uint256 amount) = Math.tryAdd( + super.allowance(owner, spender), + _loadTemporaryAllowance(owner, spender) + ); + return success ? amount : type(uint256).max; + } + + /** + * @dev Alternative to {approve} that sets a `value` amount of tokens as the temporary allowance of `spender` over + * the caller's tokens. + * + * Returns a boolean value indicating whether the operation succeeded. + * + * Does NOT emit an {Approval} event. + */ + function temporaryApprove(address spender, uint256 value) public virtual returns (bool) { + address owner = _msgSender(); + _storeTemporaryAllowance(owner, spender, value); + return true; + } + + /** + * @dev {_spendAllowance} override that consumes the temporary allowance (if any) before eventually falling back + * onto the the transitional, persistent, allowance. + * + * In case the temporary allowance is enough to cover the required value, this will NOT make a super call. This + * should be considered in case this extension is used with code that also overrides {_spendAllowance}. + */ + function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { + unchecked { + // load transient allowance + uint256 currentTemporaryAllowance = _loadTemporaryAllowance(owner, spender); + // if there is temporary allowance + if (currentTemporaryAllowance > 0) { + // if infinite, do nothing + if (currentTemporaryAllowance == type(uint256).max) return; + // check how much of the value is covered by the transient allowance + uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value); + // decrease transient allowance accordingly + _storeTemporaryAllowance(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); + // update value necessary + value -= spendTemporaryAllowance; + } + // if allowance is still needed, or if + if (value > 0) { + super._spendAllowance(owner, spender, value); + } + } + } + + function _loadTemporaryAllowance(address owner, address spender) private view returns (uint256) { + return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tload(); + } + + function _storeTemporaryAllowance(address owner, address spender, uint256 value) private { + ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tstore(value); + } +} From 4f94434b5ae5b5d51dca442f83c49641f9b25421 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 16:50:03 +0200 Subject: [PATCH 02/23] Update .changeset/serious-carrots-provide.md --- .changeset/serious-carrots-provide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md index 59e58a7ed05..af7c0a466ed 100644 --- a/.changeset/serious-carrots-provide.md +++ b/.changeset/serious-carrots-provide.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC20TemporaryAllowance`: add an ERC-20 extension that implements temporary approval using transient storage +`ERC20TemporaryApproval`: add an ERC-20 extension that implements temporary approval using transient storage From 119f7189ccf934f5ee40d61467edd2dc2bf432a8 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 18:25:16 +0200 Subject: [PATCH 03/23] testing --- contracts/mocks/BatchCaller.sol | 20 ++ contracts/mocks/token/ERC20GetterHelper.sol | 38 ++++ contracts/token/ERC20/ERC20.sol | 16 +- .../draft-ERC20TemporaryApproval.sol | 13 +- test/token/ERC20/ERC20.behavior.js | 2 +- .../extensions/ERC20TemporaryApproval.test.js | 196 ++++++++++++++++++ 6 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 contracts/mocks/BatchCaller.sol create mode 100644 contracts/mocks/token/ERC20GetterHelper.sol create mode 100644 test/token/ERC20/extensions/ERC20TemporaryApproval.test.js diff --git a/contracts/mocks/BatchCaller.sol b/contracts/mocks/BatchCaller.sol new file mode 100644 index 00000000000..740691ba4f0 --- /dev/null +++ b/contracts/mocks/BatchCaller.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Address} from "../utils/Address.sol"; + +contract BatchCaller { + struct Call { + address target; + uint256 value; + bytes data; + } + + function execute(Call[] calldata calls) external returns (bytes[] memory) { + bytes[] memory returndata = new bytes[](calls.length); + for (uint256 i = 0; i < calls.length; ++i) { + returndata[i] = Address.functionCallWithValue(calls[i].target, calls[i].data, calls[i].value); + } + return returndata; + } +} diff --git a/contracts/mocks/token/ERC20GetterHelper.sol b/contracts/mocks/token/ERC20GetterHelper.sol new file mode 100644 index 00000000000..26c1213e6ae --- /dev/null +++ b/contracts/mocks/token/ERC20GetterHelper.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IERC20} from "../../token/ERC20/IERC20.sol"; +import {IERC20Metadata} from "../../token/ERC20/extensions/IERC20Metadata.sol"; + +contract ERC20GetterHelper { + event erc20totalSupply(IERC20 token, uint256 totalSupply); + event erc20balanceOf(IERC20 token, address account, uint256 balanceOf); + event erc20allowance(IERC20 token, address owner, address spender, uint256 allowance); + event erc20name(IERC20Metadata token, string name); + event erc20symbol(IERC20Metadata token, string symbol); + event erc20decimals(IERC20Metadata token, uint8 decimals); + + function totalSupply(IERC20 token) external { + emit erc20totalSupply(token, token.totalSupply()); + } + + function balanceOf(IERC20 token, address account) external { + emit erc20balanceOf(token, account, token.balanceOf(account)); + } + + function allowance(IERC20 token, address owner, address spender) external { + emit erc20allowance(token, owner, spender, token.allowance(owner, spender)); + } + + function name(IERC20Metadata token) external { + emit erc20name(token, token.name()); + } + + function symbol(IERC20Metadata token) external { + emit erc20symbol(token, token.symbol()); + } + + function decimals(IERC20Metadata token) external { + emit erc20decimals(token, token.decimals()); + } +} diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 9885eaac996..d828e3138d0 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -299,13 +299,15 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors { * Does not emit an {Approval} event. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual { - uint256 currentAllowance = allowance(owner, spender); - if (currentAllowance != type(uint256).max) { - if (currentAllowance < value) { - revert ERC20InsufficientAllowance(spender, currentAllowance, value); - } - unchecked { - _approve(owner, spender, currentAllowance - value, false); + if (value > 0) { + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + if (currentAllowance < value) { + revert ERC20InsufficientAllowance(spender, currentAllowance, value); + } + unchecked { + _approve(owner, spender, currentAllowance - value, false); + } } } } diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index 515e1150222..ab17cea28b7 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -24,7 +24,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { /** * @dev {allowance} override that includes the temporary allowance when looking up the current allowance. If - * adding up the "traditional" and the temporary allowances result in an overflow, type(uint256).max is returned. + * adding up the persistent and the temporary allowances result in an overflow, type(uint256).max is returned. */ function allowance(address owner, address spender) public view virtual override returns (uint256) { (bool success, uint256 amount) = Math.tryAdd( @@ -50,10 +50,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { /** * @dev {_spendAllowance} override that consumes the temporary allowance (if any) before eventually falling back - * onto the the transitional, persistent, allowance. - * - * In case the temporary allowance is enough to cover the required value, this will NOT make a super call. This - * should be considered in case this extension is used with code that also overrides {_spendAllowance}. + * to consumming the persistent allowance. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { unchecked { @@ -70,10 +67,8 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { // update value necessary value -= spendTemporaryAllowance; } - // if allowance is still needed, or if - if (value > 0) { - super._spendAllowance(owner, spender, value); - } + // reduce any remaining value from the persistent allowance + super._spendAllowance(owner, spender, value); } } diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 6754bff336a..63dafe67d56 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -134,7 +134,7 @@ function shouldBehaveLikeERC20(initialSupply, opts = {}) { it('reverts when the token owner is the zero address', async function () { const value = 0n; await expect(this.token.connect(this.recipient).transferFrom(ethers.ZeroAddress, this.recipient, value)) - .to.be.revertedWithCustomError(this.token, 'ERC20InvalidApprover') + .to.be.revertedWithCustomError(this.token, 'ERC20InvalidSender') .withArgs(ethers.ZeroAddress); }); }); diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js new file mode 100644 index 00000000000..42995914985 --- /dev/null +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -0,0 +1,196 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); + +const { shouldBehaveLikeERC20 } = require('../ERC20.behavior.js'); + +const name = 'My Token'; +const symbol = 'MTKN'; +const initialSupply = 100n; + +async function fixture() { + // this.accounts is used by shouldBehaveLikeERC20 + const accounts = await ethers.getSigners(); + const [holder, recipient, other] = accounts; + + const token = await ethers.deployContract('$ERC20TemporaryApproval', [name, symbol]); + await token.$_mint(holder, initialSupply); + + const spender = await ethers.deployContract('$Address'); + const batch = await ethers.deployContract('BatchCaller'); + const getter = await ethers.deployContract('ERC20GetterHelper'); + + return { accounts, holder, recipient, other, token, spender, batch, getter }; +} + +describe('ERC20TemporaryApproval', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + shouldBehaveLikeERC20(initialSupply); + + describe('setting temporary allowance', function () { + const persistentAmount = 42n; + const temporaryAmount = 17n; + + it('can set temporary allowance', async function () { + await expect( + this.batch.execute([ + { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, temporaryAmount]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ]), + ) + .to.emit(this.getter, 'erc20allowance') + .withArgs(this.token, this.batch, this.spender, temporaryAmount); + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); + }); + + it('can set temporary allowance on top of persistent allowance', async function () { + await this.token.$_approve(this.batch, this.spender, persistentAmount); + + await expect( + this.batch.execute([ + { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, temporaryAmount]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ]), + ) + .to.emit(this.getter, 'erc20allowance') + .withArgs(this.token, this.batch, this.spender, persistentAmount + temporaryAmount); + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(persistentAmount); + }); + }); + + describe('spending temporary allowance', function () { + beforeEach(async function () { + await this.token.connect(this.holder).transfer(this.batch, initialSupply); + }); + + it('consumming temporary allowance alone', async function () { + await expect( + this.batch.execute([ + { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, 10n]), + }, + { + target: this.spender, + value: 0n, + data: this.spender.interface.encodeFunctionData('$functionCall', [ + this.token.target, + this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), + ]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ]), + ) + .to.emit(this.getter, 'erc20allowance') + .withArgs(this.token, this.batch, this.spender, 8n); // 10 - 2 + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); + }); + + it('do not reduce infinite temporary allowance', async function () { + await expect( + this.batch.execute([ + { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, ethers.MaxUint256]), + }, + { + target: this.spender, + value: 0n, + data: this.spender.interface.encodeFunctionData('$functionCall', [ + this.token.target, + this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), + ]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ]), + ) + .to.emit(this.getter, 'erc20allowance') + .withArgs(this.token, this.batch, this.spender, ethers.MaxUint256); + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); + }); + + it('fallback to persistent allowance if temporary allowance is not sufficient', async function () { + await this.token.$_approve(this.batch, this.spender, 10n); + + await expect( + this.batch.execute([ + { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, 1n]), + }, + { + target: this.spender, + value: 0n, + data: this.spender.interface.encodeFunctionData('$functionCall', [ + this.token.target, + this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), + ]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ]), + ) + .to.emit(this.getter, 'erc20allowance') + .withArgs(this.token, this.batch, this.spender, 9n); // 10 + 1 - 2 + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(9n); // 10 - 1 + }); + }); +}); From 76f17453c17fe4e80a9dd068e622e00050a5f0ba Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 19:02:28 +0200 Subject: [PATCH 04/23] update --- contracts/mocks/token/ERC20GetterHelper.sol | 24 +- .../draft-ERC20TemporaryApproval.sol | 50 +++- .../extensions/ERC20TemporaryApproval.test.js | 250 +++++++----------- 3 files changed, 141 insertions(+), 183 deletions(-) diff --git a/contracts/mocks/token/ERC20GetterHelper.sol b/contracts/mocks/token/ERC20GetterHelper.sol index 26c1213e6ae..5f539fcd55a 100644 --- a/contracts/mocks/token/ERC20GetterHelper.sol +++ b/contracts/mocks/token/ERC20GetterHelper.sol @@ -5,34 +5,34 @@ import {IERC20} from "../../token/ERC20/IERC20.sol"; import {IERC20Metadata} from "../../token/ERC20/extensions/IERC20Metadata.sol"; contract ERC20GetterHelper { - event erc20totalSupply(IERC20 token, uint256 totalSupply); - event erc20balanceOf(IERC20 token, address account, uint256 balanceOf); - event erc20allowance(IERC20 token, address owner, address spender, uint256 allowance); - event erc20name(IERC20Metadata token, string name); - event erc20symbol(IERC20Metadata token, string symbol); - event erc20decimals(IERC20Metadata token, uint8 decimals); + event ERC20totalSupply(IERC20 token, uint256 totalSupply); + event ERC20balanceOf(IERC20 token, address account, uint256 balanceOf); + event ERC20allowance(IERC20 token, address owner, address spender, uint256 allowance); + event ERC20name(IERC20Metadata token, string name); + event ERC20symbol(IERC20Metadata token, string symbol); + event ERC20decimals(IERC20Metadata token, uint8 decimals); function totalSupply(IERC20 token) external { - emit erc20totalSupply(token, token.totalSupply()); + emit ERC20totalSupply(token, token.totalSupply()); } function balanceOf(IERC20 token, address account) external { - emit erc20balanceOf(token, account, token.balanceOf(account)); + emit ERC20balanceOf(token, account, token.balanceOf(account)); } function allowance(IERC20 token, address owner, address spender) external { - emit erc20allowance(token, owner, spender, token.allowance(owner, spender)); + emit ERC20allowance(token, owner, spender, token.allowance(owner, spender)); } function name(IERC20Metadata token) external { - emit erc20name(token, token.name()); + emit ERC20name(token, token.name()); } function symbol(IERC20Metadata token) external { - emit erc20symbol(token, token.symbol()); + emit ERC20symbol(token, token.symbol()); } function decimals(IERC20Metadata token) external { - emit erc20decimals(token, token.decimals()); + emit ERC20decimals(token, token.decimals()); } } diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index ab17cea28b7..57b7b83115c 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -19,6 +19,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { using StorageSlot for StorageSlot.Uint256SlotType; // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20TemporaryApproval")) - 1)) & ~bytes32(uint256(0xff)) + // solhint-disable-next-line const-name-snakecase bytes32 private constant ERC20TemporaryApprovalStorageLocation = 0x0fd66af0be6cb88466bb5c49c7ea8fbb4acdc82057e863d0a17fddeaaf18fe00; @@ -29,25 +30,56 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { function allowance(address owner, address spender) public view virtual override returns (uint256) { (bool success, uint256 amount) = Math.tryAdd( super.allowance(owner, spender), - _loadTemporaryAllowance(owner, spender) + _temporaryAllowance(owner, spender) ); return success ? amount : type(uint256).max; } + /** + * @dev Internal getter for the current temporary allowance that `spender` has over `owner` tokens. + */ + function _temporaryAllowance(address owner, address spender) internal view virtual returns (uint256) { + return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tload(); + } + /** * @dev Alternative to {approve} that sets a `value` amount of tokens as the temporary allowance of `spender` over * the caller's tokens. * * Returns a boolean value indicating whether the operation succeeded. * + * Requirements: + * - `spender` cannot be the zero address. + * * Does NOT emit an {Approval} event. */ function temporaryApprove(address spender, uint256 value) public virtual returns (bool) { - address owner = _msgSender(); - _storeTemporaryAllowance(owner, spender, value); + _temporaryApprove(_msgSender(), spender, value); return true; } + /** + * @dev Sets `value` as the temporary allowance of `spender` over the `owner` s tokens. + * + * This internal function is equivalent to `temporaryApprove`, and can be used to e.g. set automatic allowances + * for certain subsystems, etc. + * + * Requirements: + * - `owner` cannot be the zero address. + * - `spender` cannot be the zero address. + * + * Does NOT emit an {Approval} event. + */ + function _temporaryApprove(address owner, address spender, uint256 value) internal virtual { + if (owner == address(0)) { + revert ERC20InvalidApprover(address(0)); + } + if (spender == address(0)) { + revert ERC20InvalidSpender(address(0)); + } + ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tstore(value); + } + /** * @dev {_spendAllowance} override that consumes the temporary allowance (if any) before eventually falling back * to consumming the persistent allowance. @@ -55,7 +87,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { unchecked { // load transient allowance - uint256 currentTemporaryAllowance = _loadTemporaryAllowance(owner, spender); + uint256 currentTemporaryAllowance = _temporaryAllowance(owner, spender); // if there is temporary allowance if (currentTemporaryAllowance > 0) { // if infinite, do nothing @@ -63,7 +95,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { // check how much of the value is covered by the transient allowance uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value); // decrease transient allowance accordingly - _storeTemporaryAllowance(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); + _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); // update value necessary value -= spendTemporaryAllowance; } @@ -71,12 +103,4 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { super._spendAllowance(owner, spender, value); } } - - function _loadTemporaryAllowance(address owner, address spender) private view returns (uint256) { - return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tload(); - } - - function _storeTemporaryAllowance(address owner, address spender, uint256 value) private { - ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tstore(value); - } } diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js index 42995914985..6b35c33695c 100644 --- a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -1,6 +1,7 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { max, min } = require('../../../helpers/math.js'); const { shouldBehaveLikeERC20 } = require('../ERC20.behavior.js'); @@ -30,167 +31,100 @@ describe('ERC20TemporaryApproval', function () { shouldBehaveLikeERC20(initialSupply); - describe('setting temporary allowance', function () { - const persistentAmount = 42n; - const temporaryAmount = 17n; - - it('can set temporary allowance', async function () { - await expect( - this.batch.execute([ - { - target: this.token, - value: 0n, - data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, temporaryAmount]), - }, - { - target: this.getter, - value: 0n, - data: this.getter.interface.encodeFunctionData('allowance', [ - this.token.target, - this.batch.target, - this.spender.target, - ]), - }, - ]), - ) - .to.emit(this.getter, 'erc20allowance') - .withArgs(this.token, this.batch, this.spender, temporaryAmount); - - expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); - }); - - it('can set temporary allowance on top of persistent allowance', async function () { - await this.token.$_approve(this.batch, this.spender, persistentAmount); - - await expect( - this.batch.execute([ - { - target: this.token, - value: 0n, - data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, temporaryAmount]), - }, - { - target: this.getter, - value: 0n, - data: this.getter.interface.encodeFunctionData('allowance', [ - this.token.target, - this.batch.target, - this.spender.target, - ]), - }, - ]), - ) - .to.emit(this.getter, 'erc20allowance') - .withArgs(this.token, this.batch, this.spender, persistentAmount + temporaryAmount); - - expect(await this.token.allowance(this.batch, this.spender)).to.equal(persistentAmount); - }); - }); - - describe('spending temporary allowance', function () { + describe('setting and spending temporary allowance', function () { beforeEach(async function () { await this.token.connect(this.holder).transfer(this.batch, initialSupply); }); - it('consumming temporary allowance alone', async function () { - await expect( - this.batch.execute([ - { - target: this.token, - value: 0n, - data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, 10n]), - }, - { - target: this.spender, - value: 0n, - data: this.spender.interface.encodeFunctionData('$functionCall', [ - this.token.target, - this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), - ]), - }, - { - target: this.getter, - value: 0n, - data: this.getter.interface.encodeFunctionData('allowance', [ - this.token.target, - this.batch.target, - this.spender.target, - ]), - }, - ]), - ) - .to.emit(this.getter, 'erc20allowance') - .withArgs(this.token, this.batch, this.spender, 8n); // 10 - 2 - - expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); - }); - - it('do not reduce infinite temporary allowance', async function () { - await expect( - this.batch.execute([ - { - target: this.token, - value: 0n, - data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, ethers.MaxUint256]), - }, - { - target: this.spender, - value: 0n, - data: this.spender.interface.encodeFunctionData('$functionCall', [ - this.token.target, - this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), - ]), - }, - { - target: this.getter, - value: 0n, - data: this.getter.interface.encodeFunctionData('allowance', [ - this.token.target, - this.batch.target, - this.spender.target, - ]), - }, - ]), - ) - .to.emit(this.getter, 'erc20allowance') - .withArgs(this.token, this.batch, this.spender, ethers.MaxUint256); - - expect(await this.token.allowance(this.batch, this.spender)).to.equal(0n); - }); - - it('fallback to persistent allowance if temporary allowance is not sufficient', async function () { - await this.token.$_approve(this.batch, this.spender, 10n); - - await expect( - this.batch.execute([ - { - target: this.token, - value: 0n, - data: this.token.interface.encodeFunctionData('temporaryApprove', [this.spender.target, 1n]), - }, - { - target: this.spender, - value: 0n, - data: this.spender.interface.encodeFunctionData('$functionCall', [ - this.token.target, - this.token.interface.encodeFunctionData('transferFrom', [this.batch.target, this.recipient.address, 2n]), - ]), - }, - { - target: this.getter, - value: 0n, - data: this.getter.interface.encodeFunctionData('allowance', [ - this.token.target, - this.batch.target, - this.spender.target, - ]), - }, - ]), - ) - .to.emit(this.getter, 'erc20allowance') - .withArgs(this.token, this.batch, this.spender, 9n); // 10 + 1 - 2 - - expect(await this.token.allowance(this.batch, this.spender)).to.equal(9n); // 10 - 1 - }); + for (let { + description, + persistentAllowance, + temporaryAllowance, + amount, + temporaryExpected, + persistentExpected, + } of [ + { description: 'can set temporary allowance', temporaryAllowance: 42n }, + { + description: 'can set temporary allowance on top of persistent allowance', + temporaryAllowance: 42n, + persistentAllowance: 17n, + }, + { description: 'support allowance overflow', temporaryAllowance: ethers.MaxUint256, persistentAllowance: 17n }, + { description: 'consumming temporary allowance alone', temporaryAllowance: 42n, amount: 2n }, + { + description: 'fallback to persistent allowance if temporary allowance is not sufficient', + temporaryAllowance: 42n, + persistentAllowance: 17n, + amount: 50n, + }, + { + description: 'do not reduce infinite temporary allowance #1', + temporaryAllowance: ethers.MaxUint256, + amount: 50n, + temporaryExpected: ethers.MaxUint256, + }, + { + description: 'do not reduce infinite temporary allowance #2', + temporaryAllowance: 17n, + persistentAllowance: ethers.MaxUint256, + amount: 50n, + temporaryExpected: ethers.MaxUint256, + persistentExpected: ethers.MaxUint256, + }, + ]) { + persistentAllowance ??= 0n; + temporaryAllowance ??= 0n; + amount ??= 0n; + temporaryExpected ??= min(persistentAllowance + temporaryAllowance - amount, ethers.MaxUint256); + persistentExpected ??= persistentAllowance - max(0n, amount - temporaryAllowance); + + it(description, async function () { + await expect( + this.batch.execute( + [ + persistentAllowance && { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('approve', [this.spender.target, persistentAllowance]), + }, + temporaryAllowance && { + target: this.token, + value: 0n, + data: this.token.interface.encodeFunctionData('temporaryApprove', [ + this.spender.target, + temporaryAllowance, + ]), + }, + amount && { + target: this.spender, + value: 0n, + data: this.spender.interface.encodeFunctionData('$functionCall', [ + this.token.target, + this.token.interface.encodeFunctionData('transferFrom', [ + this.batch.target, + this.recipient.address, + amount, + ]), + ]), + }, + { + target: this.getter, + value: 0n, + data: this.getter.interface.encodeFunctionData('allowance', [ + this.token.target, + this.batch.target, + this.spender.target, + ]), + }, + ].filter(Boolean), + ), + ) + .to.emit(this.getter, 'ERC20allowance') + .withArgs(this.token, this.batch, this.spender, temporaryExpected); + + expect(await this.token.allowance(this.batch, this.spender)).to.equal(persistentExpected); + }); + } }); }); From 1e6911e192f620b0599a9a49ac441a001892e432 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 19:48:01 +0200 Subject: [PATCH 05/23] coverage --- .../ERC20/extensions/ERC20TemporaryApproval.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js index 6b35c33695c..aa6a7f38b35 100644 --- a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -126,5 +126,17 @@ describe('ERC20TemporaryApproval', function () { expect(await this.token.allowance(this.batch, this.spender)).to.equal(persistentExpected); }); } + + it('reverts when the recipient is the zero address', async function () { + await expect(this.token.connect(this.holder).temporaryApprove(ethers.ZeroAddress, 1n)) + .to.be.revertedWithCustomError(this.token, 'ERC20InvalidSpender') + .withArgs(ethers.ZeroAddress); + }); + + it('reverts when the token owner is the zero address', async function () { + await expect(this.token.$_temporaryApprove(ethers.ZeroAddress, this.recipient, 1n)) + .to.be.revertedWithCustomError(this.token, 'ERC20InvalidApprover') + .withArgs(ethers.ZeroAddress); + }); }); }); From 28402a6277a3458ff844094fc06f9d3b17b19c80 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 21:43:58 +0200 Subject: [PATCH 06/23] doc --- contracts/interfaces/draft-IERC7674.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 3be8bec2f8b..8e6b4ae8e9a 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -2,6 +2,15 @@ pragma solidity ^0.8.20; +/** + * @dev Temporary Approval Extension for ERC-20 (https://github.com/ethereum/ERCs/pull/358[ERC-7674]) + * + * WARNING: This ERC is not final, and is likelly to evolve. + */ interface IERC7674 { + /** + * @dev Set the temporary allowance, allowing allows `spender` to withdraw (within the same transaction) assets + * held by the caller. + */ function temporaryApprove(address spender, uint256 value) external returns (bool success); } From 43ee72774b7863bf97fcb5375f1fafd752833369 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 22:01:13 +0200 Subject: [PATCH 07/23] Update draft-IERC7674.sol --- contracts/interfaces/draft-IERC7674.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 8e6b4ae8e9a..795990624a1 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.20; /** * @dev Temporary Approval Extension for ERC-20 (https://github.com/ethereum/ERCs/pull/358[ERC-7674]) * - * WARNING: This ERC is not final, and is likelly to evolve. + * WARNING: This ERC is not final, and is likely to evolve. */ interface IERC7674 { /** From e729394462d8e0856971bb74a98ce4bd03719d81 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 22:02:50 +0200 Subject: [PATCH 08/23] Update test/token/ERC20/extensions/ERC20TemporaryApproval.test.js --- test/token/ERC20/extensions/ERC20TemporaryApproval.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js index aa6a7f38b35..f1916237695 100644 --- a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -77,7 +77,7 @@ describe('ERC20TemporaryApproval', function () { temporaryAllowance ??= 0n; amount ??= 0n; temporaryExpected ??= min(persistentAllowance + temporaryAllowance - amount, ethers.MaxUint256); - persistentExpected ??= persistentAllowance - max(0n, amount - temporaryAllowance); + persistentExpected ??= persistentAllowance - max(amount - temporaryAllowance, 0n); it(description, async function () { await expect( From 2bbd0b25ba769ee6d1055590c0331a1d7b914b14 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 7 Jun 2024 22:05:43 +0200 Subject: [PATCH 09/23] rename events in helper --- contracts/mocks/token/ERC20GetterHelper.sol | 24 +++++++++---------- .../extensions/ERC20TemporaryApproval.test.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/mocks/token/ERC20GetterHelper.sol b/contracts/mocks/token/ERC20GetterHelper.sol index 5f539fcd55a..acdcced9fa1 100644 --- a/contracts/mocks/token/ERC20GetterHelper.sol +++ b/contracts/mocks/token/ERC20GetterHelper.sol @@ -5,34 +5,34 @@ import {IERC20} from "../../token/ERC20/IERC20.sol"; import {IERC20Metadata} from "../../token/ERC20/extensions/IERC20Metadata.sol"; contract ERC20GetterHelper { - event ERC20totalSupply(IERC20 token, uint256 totalSupply); - event ERC20balanceOf(IERC20 token, address account, uint256 balanceOf); - event ERC20allowance(IERC20 token, address owner, address spender, uint256 allowance); - event ERC20name(IERC20Metadata token, string name); - event ERC20symbol(IERC20Metadata token, string symbol); - event ERC20decimals(IERC20Metadata token, uint8 decimals); + event ERC20TotalSupply(IERC20 token, uint256 totalSupply); + event ERC20BalanceOf(IERC20 token, address account, uint256 balanceOf); + event ERC20Allowance(IERC20 token, address owner, address spender, uint256 allowance); + event ERC20Name(IERC20Metadata token, string name); + event ERC20Symbol(IERC20Metadata token, string symbol); + event ERC20Decimals(IERC20Metadata token, uint8 decimals); function totalSupply(IERC20 token) external { - emit ERC20totalSupply(token, token.totalSupply()); + emit ERC20TotalSupply(token, token.totalSupply()); } function balanceOf(IERC20 token, address account) external { - emit ERC20balanceOf(token, account, token.balanceOf(account)); + emit ERC20BalanceOf(token, account, token.balanceOf(account)); } function allowance(IERC20 token, address owner, address spender) external { - emit ERC20allowance(token, owner, spender, token.allowance(owner, spender)); + emit ERC20Allowance(token, owner, spender, token.allowance(owner, spender)); } function name(IERC20Metadata token) external { - emit ERC20name(token, token.name()); + emit ERC20Name(token, token.name()); } function symbol(IERC20Metadata token) external { - emit ERC20symbol(token, token.symbol()); + emit ERC20Symbol(token, token.symbol()); } function decimals(IERC20Metadata token) external { - emit ERC20decimals(token, token.decimals()); + emit ERC20Decimals(token, token.decimals()); } } diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js index f1916237695..74551495c16 100644 --- a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -120,7 +120,7 @@ describe('ERC20TemporaryApproval', function () { ].filter(Boolean), ), ) - .to.emit(this.getter, 'ERC20allowance') + .to.emit(this.getter, 'ERC20Allowance') .withArgs(this.token, this.batch, this.spender, temporaryExpected); expect(await this.token.allowance(this.batch, this.spender)).to.equal(persistentExpected); From 580c0ec92c59886b5b01dad48747a7c136144c0a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jun 2024 13:12:14 +0200 Subject: [PATCH 10/23] update --- .changeset/serious-carrots-provide.md | 2 +- contracts/interfaces/draft-IERC7674.sol | 2 -- contracts/token/ERC20/README.adoc | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md index af7c0a466ed..589458112e2 100644 --- a/.changeset/serious-carrots-provide.md +++ b/.changeset/serious-carrots-provide.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC20TemporaryApproval`: add an ERC-20 extension that implements temporary approval using transient storage +`ERC20TemporaryApproval`: Add an ERC-20 extension that implements temporary approval using transient storage. diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 795990624a1..8977cc9b565 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -4,8 +4,6 @@ pragma solidity ^0.8.20; /** * @dev Temporary Approval Extension for ERC-20 (https://github.com/ethereum/ERCs/pull/358[ERC-7674]) - * - * WARNING: This ERC is not final, and is likely to evolve. */ interface IERC7674 { /** diff --git a/contracts/token/ERC20/README.adoc b/contracts/token/ERC20/README.adoc index 7a5d783d299..555fc4efa31 100644 --- a/contracts/token/ERC20/README.adoc +++ b/contracts/token/ERC20/README.adoc @@ -22,7 +22,7 @@ Additionally there are multiple custom extensions, including: * {ERC20FlashMint}: token level support for flash loans through the minting and burning of ephemeral tokens (standardized as ERC-3156). * {ERC20Votes}: support for voting and vote delegation. * {ERC20Wrapper}: wrapper to create an ERC-20 backed by another ERC-20, with deposit and withdraw methods. Useful in conjunction with {ERC20Votes}. -* {ERC20TemporaryApproval}: support for temporary allowances following ERC-7674. +* {ERC20TemporaryApproval}: support for approvals lasting for only one transaction, as defined in ERC-7674. * {ERC1363}: support for calling the target of a transfer or approval, enabling code execution on the receiver within a single transaction. * {ERC4626}: tokenized vault that manages shares (represented as ERC-20) that are backed by assets (another ERC-20). From f48694cf85ee0daac10a967a4f59c4f6d9b42209 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jun 2024 13:26:13 +0200 Subject: [PATCH 11/23] update --- .../draft-ERC20TemporaryApproval.sol | 41 ++++++++++++------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index 57b7b83115c..7a74f371655 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -39,7 +39,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { * @dev Internal getter for the current temporary allowance that `spender` has over `owner` tokens. */ function _temporaryAllowance(address owner, address spender) internal view virtual returns (uint256) { - return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tload(); + return _temporaryAllowanceSlot(owner, spender).tload(); } /** @@ -77,7 +77,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { if (spender == address(0)) { revert ERC20InvalidSpender(address(0)); } - ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256().tstore(value); + _temporaryAllowanceSlot(owner, spender).tstore(value); } /** @@ -85,22 +85,33 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { * to consumming the persistent allowance. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { - unchecked { - // load transient allowance - uint256 currentTemporaryAllowance = _temporaryAllowance(owner, spender); - // if there is temporary allowance - if (currentTemporaryAllowance > 0) { - // if infinite, do nothing - if (currentTemporaryAllowance == type(uint256).max) return; + // load transient allowance + uint256 currentTemporaryAllowance = _temporaryAllowance(owner, spender); + + // Check and update (if needed) the temporary allowance + set remaining value + if (currentTemporaryAllowance > 0) { + if (currentTemporaryAllowance == type(uint256).max) { + // all value is covered by the infinite allowance. nothing left to spend. + value = 0; + } else { // check how much of the value is covered by the transient allowance uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value); - // decrease transient allowance accordingly - _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); - // update value necessary - value -= spendTemporaryAllowance; + unchecked { + // decrease transient allowance accordingly + _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); + // update value necessary + value -= spendTemporaryAllowance; + } } - // reduce any remaining value from the persistent allowance - super._spendAllowance(owner, spender, value); } + // reduce any remaining value from the persistent allowance + super._spendAllowance(owner, spender, value); + } + + function _temporaryAllowanceSlot( + address owner, + address spender + ) private pure returns (StorageSlot.Uint256SlotType) { + return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256(); } } From 8119a6e913c4dd4e45dc6ceaf30751bcc547890d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jun 2024 15:22:57 +0200 Subject: [PATCH 12/23] rename ERC20TemporaryApproval -> ERC7674 --- .changeset/serious-carrots-provide.md | 2 +- contracts/token/ERC20/README.adoc | 6 +++--- ...ft-ERC20TemporaryApproval.sol => draft-ERC7674.sol} | 10 ++++------ ...{ERC20TemporaryApproval.test.js => ERC7674.test.js} | 4 ++-- 4 files changed, 10 insertions(+), 12 deletions(-) rename contracts/token/ERC20/extensions/{draft-ERC20TemporaryApproval.sol => draft-ERC7674.sol} (91%) rename test/token/ERC20/extensions/{ERC20TemporaryApproval.test.js => ERC7674.test.js} (97%) diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md index 589458112e2..ffc49b6473a 100644 --- a/.changeset/serious-carrots-provide.md +++ b/.changeset/serious-carrots-provide.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC20TemporaryApproval`: Add an ERC-20 extension that implements temporary approval using transient storage. +`ERC7674`: Add an ERC-20 extension that implements temporary approval using transient storage (draft). diff --git a/contracts/token/ERC20/README.adoc b/contracts/token/ERC20/README.adoc index 555fc4efa31..6487bbb2143 100644 --- a/contracts/token/ERC20/README.adoc +++ b/contracts/token/ERC20/README.adoc @@ -22,9 +22,9 @@ Additionally there are multiple custom extensions, including: * {ERC20FlashMint}: token level support for flash loans through the minting and burning of ephemeral tokens (standardized as ERC-3156). * {ERC20Votes}: support for voting and vote delegation. * {ERC20Wrapper}: wrapper to create an ERC-20 backed by another ERC-20, with deposit and withdraw methods. Useful in conjunction with {ERC20Votes}. -* {ERC20TemporaryApproval}: support for approvals lasting for only one transaction, as defined in ERC-7674. * {ERC1363}: support for calling the target of a transfer or approval, enabling code execution on the receiver within a single transaction. * {ERC4626}: tokenized vault that manages shares (represented as ERC-20) that are backed by assets (another ERC-20). +* {ERC7674}: support for approvals lasting for only one transaction, as defined in ERC-7674. Finally, there are some utilities to interact with ERC-20 contracts in various ways: @@ -62,12 +62,12 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC20Wrapper}} -{{ERC20TemporaryApproval}} - {{ERC1363}} {{ERC4626}} +{{ERC7674}} + == Utilities {{SafeERC20}} diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC7674.sol similarity index 91% rename from contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol rename to contracts/token/ERC20/extensions/draft-ERC7674.sol index 7a74f371655..6228c7c3bd5 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC7674.sol @@ -13,15 +13,13 @@ import {StorageSlot} from "../../../utils/StorageSlot.sol"; * * WARNING: This is a draft contract. The corresponding ERC is still subject to changes. */ -abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { +abstract contract ERC7674 is ERC20, IERC7674 { using SlotDerivation for bytes32; using StorageSlot for bytes32; using StorageSlot for StorageSlot.Uint256SlotType; - // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20TemporaryApproval")) - 1)) & ~bytes32(uint256(0xff)) - // solhint-disable-next-line const-name-snakecase - bytes32 private constant ERC20TemporaryApprovalStorageLocation = - 0x0fd66af0be6cb88466bb5c49c7ea8fbb4acdc82057e863d0a17fddeaaf18fe00; + // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC7674")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant ERC7674_STORAGE = 0xf9459e0e3709f2b5aaa798666bbb2eb0d06d2c3958e71e4a32973e445fe1ce00; /** * @dev {allowance} override that includes the temporary allowance when looking up the current allowance. If @@ -112,6 +110,6 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { address owner, address spender ) private pure returns (StorageSlot.Uint256SlotType) { - return ERC20TemporaryApprovalStorageLocation.deriveMapping(owner).deriveMapping(spender).asUint256(); + return ERC7674_STORAGE.deriveMapping(owner).deriveMapping(spender).asUint256(); } } diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/ERC7674.test.js similarity index 97% rename from test/token/ERC20/extensions/ERC20TemporaryApproval.test.js rename to test/token/ERC20/extensions/ERC7674.test.js index 74551495c16..69e6a10289e 100644 --- a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/ERC7674.test.js @@ -14,7 +14,7 @@ async function fixture() { const accounts = await ethers.getSigners(); const [holder, recipient, other] = accounts; - const token = await ethers.deployContract('$ERC20TemporaryApproval', [name, symbol]); + const token = await ethers.deployContract('$ERC7674', [name, symbol]); await token.$_mint(holder, initialSupply); const spender = await ethers.deployContract('$Address'); @@ -24,7 +24,7 @@ async function fixture() { return { accounts, holder, recipient, other, token, spender, batch, getter }; } -describe('ERC20TemporaryApproval', function () { +describe('ERC7674', function () { beforeEach(async function () { Object.assign(this, await loadFixture(fixture)); }); From b8fa0577ff99760cd36cfe5f861bd28137f5b079 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jun 2024 20:48:24 +0200 Subject: [PATCH 13/23] rename --- .changeset/serious-carrots-provide.md | 2 +- contracts/token/ERC20/README.adoc | 10 +++++----- ...ft-ERC7674.sol => draft-ERC20TemporaryApproval.sol} | 9 +++++---- ...{ERC7674.test.js => ERC20TemporaryApproval.test.js} | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) rename contracts/token/ERC20/extensions/{draft-ERC7674.sol => draft-ERC20TemporaryApproval.sol} (92%) rename test/token/ERC20/extensions/{ERC7674.test.js => ERC20TemporaryApproval.test.js} (97%) diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md index ffc49b6473a..f64221d95b5 100644 --- a/.changeset/serious-carrots-provide.md +++ b/.changeset/serious-carrots-provide.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC7674`: Add an ERC-20 extension that implements temporary approval using transient storage (draft). +`ERC20TemporaryApproval`: Add an ERC-20 extension that implements temporary approval using transient storage (draft). diff --git a/contracts/token/ERC20/README.adoc b/contracts/token/ERC20/README.adoc index 6487bbb2143..faaacc57667 100644 --- a/contracts/token/ERC20/README.adoc +++ b/contracts/token/ERC20/README.adoc @@ -22,9 +22,9 @@ Additionally there are multiple custom extensions, including: * {ERC20FlashMint}: token level support for flash loans through the minting and burning of ephemeral tokens (standardized as ERC-3156). * {ERC20Votes}: support for voting and vote delegation. * {ERC20Wrapper}: wrapper to create an ERC-20 backed by another ERC-20, with deposit and withdraw methods. Useful in conjunction with {ERC20Votes}. +* {ERC20TemporaryApproval}: support for approvals lasting for only one transaction, as defined in ERC-7674. * {ERC1363}: support for calling the target of a transfer or approval, enabling code execution on the receiver within a single transaction. * {ERC4626}: tokenized vault that manages shares (represented as ERC-20) that are backed by assets (another ERC-20). -* {ERC7674}: support for approvals lasting for only one transaction, as defined in ERC-7674. Finally, there are some utilities to interact with ERC-20 contracts in various ways: @@ -56,18 +56,18 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC20Pausable}} -{{ERC20FlashMint}} - {{ERC20Votes}} {{ERC20Wrapper}} +{{ERC20FlashMint}} + +{{ERC20TemporaryApproval}} + {{ERC1363}} {{ERC4626}} -{{ERC7674}} - == Utilities {{SafeERC20}} diff --git a/contracts/token/ERC20/extensions/draft-ERC7674.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol similarity index 92% rename from contracts/token/ERC20/extensions/draft-ERC7674.sol rename to contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index 6228c7c3bd5..ef3aba197eb 100644 --- a/contracts/token/ERC20/extensions/draft-ERC7674.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -13,13 +13,14 @@ import {StorageSlot} from "../../../utils/StorageSlot.sol"; * * WARNING: This is a draft contract. The corresponding ERC is still subject to changes. */ -abstract contract ERC7674 is ERC20, IERC7674 { +abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { using SlotDerivation for bytes32; using StorageSlot for bytes32; using StorageSlot for StorageSlot.Uint256SlotType; - // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC7674")) - 1)) & ~bytes32(uint256(0xff)) - bytes32 private constant ERC7674_STORAGE = 0xf9459e0e3709f2b5aaa798666bbb2eb0d06d2c3958e71e4a32973e445fe1ce00; + // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20_TEMPORARY_APPROVAL_STORAGE")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant ERC20_TEMPORARY_APPROVAL_STORAGE = + 0xea2d0e77a01400d0111492b1321103eed560d8fe44b9a7c2410407714583c400; /** * @dev {allowance} override that includes the temporary allowance when looking up the current allowance. If @@ -110,6 +111,6 @@ abstract contract ERC7674 is ERC20, IERC7674 { address owner, address spender ) private pure returns (StorageSlot.Uint256SlotType) { - return ERC7674_STORAGE.deriveMapping(owner).deriveMapping(spender).asUint256(); + return ERC20_TEMPORARY_APPROVAL_STORAGE.deriveMapping(owner).deriveMapping(spender).asUint256(); } } diff --git a/test/token/ERC20/extensions/ERC7674.test.js b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js similarity index 97% rename from test/token/ERC20/extensions/ERC7674.test.js rename to test/token/ERC20/extensions/ERC20TemporaryApproval.test.js index 69e6a10289e..74551495c16 100644 --- a/test/token/ERC20/extensions/ERC7674.test.js +++ b/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js @@ -14,7 +14,7 @@ async function fixture() { const accounts = await ethers.getSigners(); const [holder, recipient, other] = accounts; - const token = await ethers.deployContract('$ERC7674', [name, symbol]); + const token = await ethers.deployContract('$ERC20TemporaryApproval', [name, symbol]); await token.$_mint(holder, initialSupply); const spender = await ethers.deployContract('$Address'); @@ -24,7 +24,7 @@ async function fixture() { return { accounts, holder, recipient, other, token, spender, batch, getter }; } -describe('ERC7674', function () { +describe('ERC20TemporaryApproval', function () { beforeEach(async function () { Object.assign(this, await loadFixture(fixture)); }); From 5bb76626d5380af3eb69dbfc24b7d1d4d4cb5bec Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 13 Jun 2024 14:57:24 +0200 Subject: [PATCH 14/23] if value greater than zero check in ERC20TemporaryApproval --- contracts/token/ERC20/ERC20.sol | 16 ++++++------ .../draft-ERC20TemporaryApproval.sol | 25 ++++++++++--------- ...s => draft-ERC20TemporaryApproval.test.js} | 0 3 files changed, 20 insertions(+), 21 deletions(-) rename test/token/ERC20/extensions/{ERC20TemporaryApproval.test.js => draft-ERC20TemporaryApproval.test.js} (100%) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index d828e3138d0..9885eaac996 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -299,15 +299,13 @@ abstract contract ERC20 is Context, IERC20, IERC20Metadata, IERC20Errors { * Does not emit an {Approval} event. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual { - if (value > 0) { - uint256 currentAllowance = allowance(owner, spender); - if (currentAllowance != type(uint256).max) { - if (currentAllowance < value) { - revert ERC20InsufficientAllowance(spender, currentAllowance, value); - } - unchecked { - _approve(owner, spender, currentAllowance - value, false); - } + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + if (currentAllowance < value) { + revert ERC20InsufficientAllowance(spender, currentAllowance, value); + } + unchecked { + _approve(owner, spender, currentAllowance - value, false); } } } diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index ef3aba197eb..6271f215039 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -89,22 +89,23 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { // Check and update (if needed) the temporary allowance + set remaining value if (currentTemporaryAllowance > 0) { + // All value is covered by the infinite allowance. nothing left to spend, we can return early if (currentTemporaryAllowance == type(uint256).max) { - // all value is covered by the infinite allowance. nothing left to spend. - value = 0; - } else { - // check how much of the value is covered by the transient allowance - uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value); - unchecked { - // decrease transient allowance accordingly - _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); - // update value necessary - value -= spendTemporaryAllowance; - } + return; + } + // check how much of the value is covered by the transient allowance + uint256 spendTemporaryAllowance = Math.min(currentTemporaryAllowance, value); + unchecked { + // decrease transient allowance accordingly + _temporaryApprove(owner, spender, currentTemporaryAllowance - spendTemporaryAllowance); + // update value necessary + value -= spendTemporaryAllowance; } } // reduce any remaining value from the persistent allowance - super._spendAllowance(owner, spender, value); + if (value > 0) { + super._spendAllowance(owner, spender, value); + } } function _temporaryAllowanceSlot( diff --git a/test/token/ERC20/extensions/ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js similarity index 100% rename from test/token/ERC20/extensions/ERC20TemporaryApproval.test.js rename to test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js From fc031f1d79695d3234d7b3101287c282c7418ac4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 13 Jun 2024 15:30:19 +0200 Subject: [PATCH 15/23] fix tests --- test/token/ERC20/ERC20.behavior.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 63dafe67d56..0582032bfb0 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -132,9 +132,18 @@ function shouldBehaveLikeERC20(initialSupply, opts = {}) { }); it('reverts when the token owner is the zero address', async function () { + // transferFrom does a spendAllowance before moving the assets + // - default behavior (ERC20) is to set the update the approval, which will fail because the approver is + // address(0) that happens even if the amount transfered is zero, and the approval update is not actually + // necessary + // - in ERC20TemporaryAllowance, transfer of 0 value will not update allowance (termporary or persisten) + // therefore the spend allowance does not revert. The transfer of asset will however revert because the sender + // is address(0) + const errorName = this.token.temporaryApprove ? 'ERC20InvalidSender' : 'ERC20InvalidApprover'; + const value = 0n; await expect(this.token.connect(this.recipient).transferFrom(ethers.ZeroAddress, this.recipient, value)) - .to.be.revertedWithCustomError(this.token, 'ERC20InvalidSender') + .to.be.revertedWithCustomError(this.token, errorName) .withArgs(ethers.ZeroAddress); }); }); From 112c22be4af52b32461f8d6cc6e76730864bd561 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 13 Jun 2024 16:52:38 +0200 Subject: [PATCH 16/23] spelling --- test/token/ERC20/ERC20.behavior.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index 0582032bfb0..b8ca6da96e0 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -133,11 +133,11 @@ function shouldBehaveLikeERC20(initialSupply, opts = {}) { it('reverts when the token owner is the zero address', async function () { // transferFrom does a spendAllowance before moving the assets - // - default behavior (ERC20) is to set the update the approval, which will fail because the approver is - // address(0) that happens even if the amount transfered is zero, and the approval update is not actually - // necessary - // - in ERC20TemporaryAllowance, transfer of 0 value will not update allowance (termporary or persisten) - // therefore the spend allowance does not revert. The transfer of asset will however revert because the sender + // - default behavior (ERC20) is to always update the approval using `_approve`. This will fail because the + // approver (owner) is address(0). This happens even if the amount transfered is zero, and the approval update + // is not actually necessary. + // - in ERC20TemporaryAllowance, transfer of 0 value will not update allowance (temporary or persistent) + // therefore the spendAllowance does not revert. However, the transfer of asset will revert because the sender // is address(0) const errorName = this.token.temporaryApprove ? 'ERC20InvalidSender' : 'ERC20InvalidApprover'; From 6f3df410762ef1052185ef432ff3142e9a673522 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 13 Jun 2024 17:48:27 +0200 Subject: [PATCH 17/23] Update test/token/ERC20/ERC20.behavior.js --- test/token/ERC20/ERC20.behavior.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC20/ERC20.behavior.js b/test/token/ERC20/ERC20.behavior.js index b8ca6da96e0..748df4b85ae 100644 --- a/test/token/ERC20/ERC20.behavior.js +++ b/test/token/ERC20/ERC20.behavior.js @@ -134,7 +134,7 @@ function shouldBehaveLikeERC20(initialSupply, opts = {}) { it('reverts when the token owner is the zero address', async function () { // transferFrom does a spendAllowance before moving the assets // - default behavior (ERC20) is to always update the approval using `_approve`. This will fail because the - // approver (owner) is address(0). This happens even if the amount transfered is zero, and the approval update + // approver (owner) is address(0). This happens even if the amount transferred is zero, and the approval update // is not actually necessary. // - in ERC20TemporaryAllowance, transfer of 0 value will not update allowance (temporary or persistent) // therefore the spendAllowance does not revert. However, the transfer of asset will revert because the sender From 352f56c8a6139ed532378dc7ddde4c55aa7fa7a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Fri, 28 Jun 2024 18:01:20 -0600 Subject: [PATCH 18/23] Apply suggestions from code review --- .../token/ERC20/extensions/draft-ERC20TemporaryApproval.sol | 2 +- .../token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index 6271f215039..0b5aa42a146 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -81,7 +81,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { /** * @dev {_spendAllowance} override that consumes the temporary allowance (if any) before eventually falling back - * to consumming the persistent allowance. + * to consuming the persistent allowance. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { // load transient allowance diff --git a/test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js b/test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js index 74551495c16..a1f6362add6 100644 --- a/test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js +++ b/test/token/ERC20/extensions/draft-ERC20TemporaryApproval.test.js @@ -51,7 +51,7 @@ describe('ERC20TemporaryApproval', function () { persistentAllowance: 17n, }, { description: 'support allowance overflow', temporaryAllowance: ethers.MaxUint256, persistentAllowance: 17n }, - { description: 'consumming temporary allowance alone', temporaryAllowance: 42n, amount: 2n }, + { description: 'consuming temporary allowance alone', temporaryAllowance: 42n, amount: 2n }, { description: 'fallback to persistent allowance if temporary allowance is not sufficient', temporaryAllowance: 42n, From a18f96350a5906b4c00d3e4a1c995c2ae8ff3d1e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 4 Jul 2024 17:15:09 +0200 Subject: [PATCH 19/23] Update contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- .../token/ERC20/extensions/draft-ERC20TemporaryApproval.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index 0b5aa42a146..f066523e38d 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -82,6 +82,8 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { /** * @dev {_spendAllowance} override that consumes the temporary allowance (if any) before eventually falling back * to consuming the persistent allowance. + * NOTE: This function skips calling `super._spendAllowance` if the temporary allowance + * is enough to cover the spending. */ function _spendAllowance(address owner, address spender, uint256 value) internal virtual override { // load transient allowance From 17f34104972780bb75b4fb025dba411e0d5fbed1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 17 Jul 2024 18:15:06 +0200 Subject: [PATCH 20/23] Apply suggestions from code review Co-authored-by: cairo --- .changeset/serious-carrots-provide.md | 2 +- contracts/interfaces/draft-IERC7674.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/serious-carrots-provide.md b/.changeset/serious-carrots-provide.md index f64221d95b5..60a16580d8a 100644 --- a/.changeset/serious-carrots-provide.md +++ b/.changeset/serious-carrots-provide.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`ERC20TemporaryApproval`: Add an ERC-20 extension that implements temporary approval using transient storage (draft). +`ERC20TemporaryApproval`: Add an ERC-20 extension that implements temporary approval using transient storage, based on ERC7674 (draft). diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 8977cc9b565..0c73e9aaf61 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -7,7 +7,7 @@ pragma solidity ^0.8.20; */ interface IERC7674 { /** - * @dev Set the temporary allowance, allowing allows `spender` to withdraw (within the same transaction) assets + * @dev Set the temporary allowance, allowing `spender` to withdraw (within the same transaction) assets * held by the caller. */ function temporaryApprove(address spender, uint256 value) external returns (bool success); From 461d527865fa14065cfdb0c28bcf791f7ff1f4fd Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 17 Jul 2024 18:16:18 +0200 Subject: [PATCH 21/23] Update draft-IERC7674.sol --- contracts/interfaces/draft-IERC7674.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 0c73e9aaf61..9702b4efc46 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -2,10 +2,12 @@ pragma solidity ^0.8.20; +import {IERC20} from "./IERC20.sol"; + /** * @dev Temporary Approval Extension for ERC-20 (https://github.com/ethereum/ERCs/pull/358[ERC-7674]) */ -interface IERC7674 { +interface IERC7674 is ERC20 { /** * @dev Set the temporary allowance, allowing `spender` to withdraw (within the same transaction) assets * held by the caller. From 12a79e75142fd074a53898b5e69708ee79aa8b11 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jul 2024 11:02:37 +0200 Subject: [PATCH 22/23] Update draft-IERC7674.sol --- contracts/interfaces/draft-IERC7674.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/interfaces/draft-IERC7674.sol b/contracts/interfaces/draft-IERC7674.sol index 9702b4efc46..949ec806e84 100644 --- a/contracts/interfaces/draft-IERC7674.sol +++ b/contracts/interfaces/draft-IERC7674.sol @@ -7,7 +7,7 @@ import {IERC20} from "./IERC20.sol"; /** * @dev Temporary Approval Extension for ERC-20 (https://github.com/ethereum/ERCs/pull/358[ERC-7674]) */ -interface IERC7674 is ERC20 { +interface IERC7674 is IERC20 { /** * @dev Set the temporary allowance, allowing `spender` to withdraw (within the same transaction) assets * held by the caller. From 887121ca264af11fa63c5d260bf649b5de349cb9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 18 Jul 2024 11:08:33 +0200 Subject: [PATCH 23/23] fix override --- .../token/ERC20/extensions/draft-ERC20TemporaryApproval.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol index f066523e38d..74da55758cb 100644 --- a/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol +++ b/contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.20; -import {ERC20} from "../ERC20.sol"; +import {IERC20, ERC20} from "../ERC20.sol"; import {IERC7674} from "../../../interfaces/draft-IERC7674.sol"; import {Math} from "../../../utils/math/Math.sol"; import {SlotDerivation} from "../../../utils/SlotDerivation.sol"; @@ -26,7 +26,7 @@ abstract contract ERC20TemporaryApproval is ERC20, IERC7674 { * @dev {allowance} override that includes the temporary allowance when looking up the current allowance. If * adding up the persistent and the temporary allowances result in an overflow, type(uint256).max is returned. */ - function allowance(address owner, address spender) public view virtual override returns (uint256) { + function allowance(address owner, address spender) public view virtual override(IERC20, ERC20) returns (uint256) { (bool success, uint256 amount) = Math.tryAdd( super.allowance(owner, spender), _temporaryAllowance(owner, spender)