From 8d47aa6049d4aba6680cf4cbf70eba89d1e87b49 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Tue, 22 Mar 2022 15:42:31 +0100
Subject: [PATCH 01/11] Add a SafeERC2O:safePermit function

---
 contracts/token/ERC20/utils/SafeERC20.sol | 25 ++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol
index 5752d93138e..3a27edc0012 100644
--- a/contracts/token/ERC20/utils/SafeERC20.sol
+++ b/contracts/token/ERC20/utils/SafeERC20.sol
@@ -4,6 +4,7 @@
 pragma solidity ^0.8.0;
 
 import "../IERC20.sol";
+import "../extensions/draft-IERC20Permit.sol";
 import "../../../utils/Address.sol";
 
 /**
@@ -23,7 +24,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.transfer.selector, to, value));
     }
 
     function safeTransferFrom(
@@ -32,7 +33,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.transferFrom.selector, from, to, value));
     }
 
     /**
@@ -54,7 +55,7 @@ library SafeERC20 {
             (value == 0) || (token.allowance(address(this), spender) == 0),
             "SafeERC20: approve from non-zero to non-zero allowance"
         );
-        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, value));
     }
 
     function safeIncreaseAllowance(
@@ -63,7 +64,7 @@ library SafeERC20 {
         uint256 value
     ) internal {
         uint256 newAllowance = token.allowance(address(this), spender) + value;
-        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, newAllowance));
     }
 
     function safeDecreaseAllowance(
@@ -75,10 +76,24 @@ library SafeERC20 {
             uint256 oldAllowance = token.allowance(address(this), spender);
             require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");
             uint256 newAllowance = oldAllowance - value;
-            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+            _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, newAllowance));
         }
     }
 
+    function safePermit(
+        IERC20 token,
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) internal {
+        _callOptionalReturn(token, abi.encodeWithSelector(IERC20Permit.permit.selector, owner, spender, value, deadline, v, r, s));
+        require(token.allowance(owner, spender) == value, "SafeERC20: permit did not set allowance");
+    }
+
     /**
      * @dev Imitates a Solidity high-level call (i.e. a regular function call to a contract), relaxing the requirement
      * on the return value: the return value is optional (but if data is returned, it must not be false).

From 40b5a46c8b8174f0325d7f7b92977794eed1c713 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Wed, 23 Mar 2022 09:31:57 +0100
Subject: [PATCH 02/11] use nonce tracking to check permit success

---
 contracts/token/ERC20/utils/SafeERC20.sol | 25 ++++++++++++++---------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol
index 3a27edc0012..1a763c771f2 100644
--- a/contracts/token/ERC20/utils/SafeERC20.sol
+++ b/contracts/token/ERC20/utils/SafeERC20.sol
@@ -24,7 +24,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.transfer.selector, to, value));
+        _callOptionalReturn(address(token), abi.encodeWithSelector(token.transfer.selector, to, value));
     }
 
     function safeTransferFrom(
@@ -33,7 +33,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.transferFrom.selector, from, to, value));
+        _callOptionalReturn(address(token), abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
     }
 
     /**
@@ -55,7 +55,7 @@ library SafeERC20 {
             (value == 0) || (token.allowance(address(this), spender) == 0),
             "SafeERC20: approve from non-zero to non-zero allowance"
         );
-        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, value));
+        _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, value));
     }
 
     function safeIncreaseAllowance(
@@ -64,7 +64,7 @@ library SafeERC20 {
         uint256 value
     ) internal {
         uint256 newAllowance = token.allowance(address(this), spender) + value;
-        _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, newAllowance));
+        _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
     }
 
     function safeDecreaseAllowance(
@@ -76,12 +76,12 @@ library SafeERC20 {
             uint256 oldAllowance = token.allowance(address(this), spender);
             require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");
             uint256 newAllowance = oldAllowance - value;
-            _callOptionalReturn(token, abi.encodeWithSelector(IERC20.approve.selector, spender, newAllowance));
+            _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
         }
     }
 
     function safePermit(
-        IERC20 token,
+        IERC20Permit token,
         address owner,
         address spender,
         uint256 value,
@@ -90,8 +90,13 @@ library SafeERC20 {
         bytes32 r,
         bytes32 s
     ) internal {
-        _callOptionalReturn(token, abi.encodeWithSelector(IERC20Permit.permit.selector, owner, spender, value, deadline, v, r, s));
-        require(token.allowance(owner, spender) == value, "SafeERC20: permit did not set allowance");
+        uint256 nonceBefore = token.nonces(owner);
+        _callOptionalReturn(
+            address(token),
+            abi.encodeWithSelector(token.permit.selector, owner, spender, value, deadline, v, r, s)
+        );
+        uint256 nonceAfter = token.nonces(owner);
+        require(nonceAfter == nonceBefore + 1, "SafeERC20: permit did not succeed");
     }
 
     /**
@@ -100,12 +105,12 @@ library SafeERC20 {
      * @param token The token targeted by the call.
      * @param data The call data (encoded using abi.encode or one of its variants).
      */
-    function _callOptionalReturn(IERC20 token, bytes memory data) private {
+    function _callOptionalReturn(address token, bytes memory data) private {
         // We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since
         // we're implementing it ourselves. We use {Address.functionCall} to perform this call, which verifies that
         // the target address contains contract code and also asserts for success in the low-level call.
 
-        bytes memory returndata = address(token).functionCall(data, "SafeERC20: low-level call failed");
+        bytes memory returndata = token.functionCall(data, "SafeERC20: low-level call failed");
         if (returndata.length > 0) {
             // Return data is optional
             require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");

From 66db182a47b051c300c885d70b8c3a151d9dcaa4 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 6 Jun 2022 11:44:19 +0200
Subject: [PATCH 03/11] IERC20Permit should extend IERC20 as described in the
 ERC

---
 .../ERC20/extensions/draft-IERC20Permit.sol      |  4 +++-
 contracts/token/ERC20/utils/SafeERC20.sol        | 16 ++++++++--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
index 6363b14084e..6572fddf6e4 100644
--- a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
+++ b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
@@ -3,6 +3,8 @@
 
 pragma solidity ^0.8.0;
 
+import "../IERC20.sol";
+
 /**
  * @dev Interface of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in
  * https://eips.ethereum.org/EIPS/eip-2612[EIP-2612].
@@ -11,7 +13,7 @@ pragma solidity ^0.8.0;
  * presenting a message signed by the account. By not relying on {IERC20-approve}, the token holder account doesn't
  * need to send a transaction, and thus is not required to hold Ether at all.
  */
-interface IERC20Permit {
+interface IERC20Permit is IERC20 {
     /**
      * @dev Sets `value` as the allowance of `spender` over ``owner``'s tokens,
      * given ``owner``'s signed approval.
diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol
index 1a763c771f2..d87d080f9ee 100644
--- a/contracts/token/ERC20/utils/SafeERC20.sol
+++ b/contracts/token/ERC20/utils/SafeERC20.sol
@@ -24,7 +24,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(address(token), abi.encodeWithSelector(token.transfer.selector, to, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value));
     }
 
     function safeTransferFrom(
@@ -33,7 +33,7 @@ library SafeERC20 {
         address to,
         uint256 value
     ) internal {
-        _callOptionalReturn(address(token), abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value));
     }
 
     /**
@@ -55,7 +55,7 @@ library SafeERC20 {
             (value == 0) || (token.allowance(address(this), spender) == 0),
             "SafeERC20: approve from non-zero to non-zero allowance"
         );
-        _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, value));
+        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
     }
 
     function safeIncreaseAllowance(
@@ -64,7 +64,7 @@ library SafeERC20 {
         uint256 value
     ) internal {
         uint256 newAllowance = token.allowance(address(this), spender) + value;
-        _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
     }
 
     function safeDecreaseAllowance(
@@ -76,7 +76,7 @@ library SafeERC20 {
             uint256 oldAllowance = token.allowance(address(this), spender);
             require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");
             uint256 newAllowance = oldAllowance - value;
-            _callOptionalReturn(address(token), abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
         }
     }
 
@@ -92,7 +92,7 @@ library SafeERC20 {
     ) internal {
         uint256 nonceBefore = token.nonces(owner);
         _callOptionalReturn(
-            address(token),
+            token,
             abi.encodeWithSelector(token.permit.selector, owner, spender, value, deadline, v, r, s)
         );
         uint256 nonceAfter = token.nonces(owner);
@@ -105,12 +105,12 @@ library SafeERC20 {
      * @param token The token targeted by the call.
      * @param data The call data (encoded using abi.encode or one of its variants).
      */
-    function _callOptionalReturn(address token, bytes memory data) private {
+    function _callOptionalReturn(IERC20 token, bytes memory data) private {
         // We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since
         // we're implementing it ourselves. We use {Address.functionCall} to perform this call, which verifies that
         // the target address contains contract code and also asserts for success in the low-level call.
 
-        bytes memory returndata = token.functionCall(data, "SafeERC20: low-level call failed");
+        bytes memory returndata = address(token).functionCall(data, "SafeERC20: low-level call failed");
         if (returndata.length > 0) {
             // Return data is optional
             require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");

From c13a9304df5f27dd3e1979a74a05db312b0d9426 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 6 Jun 2022 12:53:36 +0200
Subject: [PATCH 04/11] add safePermit tests

---
 contracts/mocks/SafeERC20Helper.sol      |  56 +++++++++-
 test/token/ERC20/utils/SafeERC20.test.js | 131 ++++++++++++++++++++++-
 2 files changed, 183 insertions(+), 4 deletions(-)

diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol
index f3bcc397261..e159f61e2ea 100644
--- a/contracts/mocks/SafeERC20Helper.sol
+++ b/contracts/mocks/SafeERC20Helper.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
 
 import "../utils/Context.sol";
 import "../token/ERC20/IERC20.sol";
+import "../token/ERC20/extensions/draft-ERC20Permit.sol";
 import "../token/ERC20/utils/SafeERC20.sol";
 
 contract ERC20ReturnFalseMock is Context {
@@ -105,12 +106,49 @@ contract ERC20NoReturnMock is Context {
     }
 }
 
+contract ERC20PermitNoRevertMock is
+    ERC20("ERC20PermitNoRevertMock", "ERC20PermitNoRevertMock"),
+    ERC20Permit("ERC20PermitNoRevertMock")
+{
+    function getChainId() external view returns (uint256) {
+        return block.chainid;
+    }
+
+    function permitRevert(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual {
+        super.permit(owner, spender, value, deadline, v, r, s);
+    }
+
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual override {
+        try this.permitRevert(owner, spender, value, deadline, v, r, s) {
+            // do nothing
+        } catch {
+            // do nothing
+        }
+    }
+}
+
 contract SafeERC20Wrapper is Context {
-    using SafeERC20 for IERC20;
+    using SafeERC20 for IERC20Permit;
 
-    IERC20 private _token;
+    IERC20Permit private _token;
 
-    constructor(IERC20 token) {
+    constructor(IERC20Permit token) {
         _token = token;
     }
 
@@ -134,6 +172,18 @@ contract SafeERC20Wrapper is Context {
         _token.safeDecreaseAllowance(address(0), amount);
     }
 
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public {
+        _token.safePermit(owner, spender, value, deadline, v, r, s);
+    }
+
     function setAllowance(uint256 allowance_) public {
         ERC20ReturnTrueMock(address(_token)).setAllowance(allowance_);
     }
diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js
index 0bed7052101..08934e6f206 100644
--- a/test/token/ERC20/utils/SafeERC20.test.js
+++ b/test/token/ERC20/utils/SafeERC20.test.js
@@ -1,10 +1,17 @@
-const { expectRevert } = require('@openzeppelin/test-helpers');
+const { constants, expectRevert } = require('@openzeppelin/test-helpers');
 
 const ERC20ReturnFalseMock = artifacts.require('ERC20ReturnFalseMock');
 const ERC20ReturnTrueMock = artifacts.require('ERC20ReturnTrueMock');
 const ERC20NoReturnMock = artifacts.require('ERC20NoReturnMock');
+const ERC20PermitNoRevertMock = artifacts.require('ERC20PermitNoRevertMock');
 const SafeERC20Wrapper = artifacts.require('SafeERC20Wrapper');
 
+const { EIP712Domain } = require('../../../helpers/eip712');
+
+const { fromRpcSig } = require('ethereumjs-util');
+const ethSigUtil = require('eth-sig-util');
+const Wallet = require('ethereumjs-wallet').default;
+
 contract('SafeERC20', function (accounts) {
   const [ hasNoCode ] = accounts;
 
@@ -39,6 +46,128 @@ contract('SafeERC20', function (accounts) {
 
     shouldOnlyRevertOnErrors();
   });
+
+  describe('with token that doesn\'t revert on invalid permit', function () {
+    const Permit = [
+      { name: 'owner', type: 'address' },
+      { name: 'spender', type: 'address' },
+      { name: 'value', type: 'uint256' },
+      { name: 'nonce', type: 'uint256' },
+      { name: 'deadline', type: 'uint256' },
+    ];
+
+    const wallet = Wallet.generate();
+    const owner = wallet.getAddressString();
+    const spender = hasNoCode;
+
+    beforeEach(async function () {
+      this.token = await ERC20PermitNoRevertMock.new();
+      this.wrapper = await SafeERC20Wrapper.new(this.token.address);
+
+      const chainId = await this.token.getChainId();
+
+      this.data = {
+        primaryType: 'Permit',
+        types: { EIP712Domain, Permit },
+        domain: { name: 'ERC20PermitNoRevertMock', version: '1', chainId, verifyingContract: this.token.address },
+        message: { owner, spender, value: '42', nonce: '0', deadline: constants.MAX_UINT256 },
+      };
+      this.signature = fromRpcSig(ethSigUtil.signTypedMessage(wallet.getPrivateKey(), { data: this.data }));
+    });
+
+    it('accepts owner signature', async function () {
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('0');
+      expect(await this.token.allowance(owner, spender)).to.be.bignumber.equal('0');
+
+      await this.wrapper.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+      expect(await this.token.allowance(owner, spender)).to.be.bignumber.equal(this.data.message.value);
+    });
+
+    it('revert on reused signature', async function () {
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('0');
+
+      await this.wrapper.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+
+      await expectRevert(
+        this.wrapper.permit(
+          this.data.message.owner,
+          this.data.message.spender,
+          this.data.message.value,
+          this.data.message.deadline,
+          this.signature.v,
+          this.signature.r,
+          this.signature.s,
+        ),
+        'SafeERC20: permit did not succeed',
+      );
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+    });
+
+    it('revert on invalid signature', async function () {
+      await expectRevert(
+        this.wrapper.permit(
+          this.data.message.owner,
+          this.data.message.spender,
+          this.data.message.value,
+          this.data.message.deadline,
+          27,
+          '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
+          '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
+        ),
+        'SafeERC20: permit did not succeed',
+      );
+    });
+
+    it('underlying token does not revert on reused or invalid signature', async function () {
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        27,
+        '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
+        '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
+      );
+    });
+  });
 });
 
 function shouldRevertOnAllCalls (reason) {

From c0cf61b0a92fe4c3d22ceb9ab2e118d11e3b36ce Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 6 Jun 2022 12:54:32 +0200
Subject: [PATCH 05/11] add Changelog entry

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f30f4174300..d7127ade2e6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@
  * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380))
  * `ERC20FlashMint`: Add customizable flash fee receiver. ([#3327](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3327))
  * `ERC20TokenizedVault`: add an extension of `ERC20` that implements the ERC4626 Tokenized Vault Standard. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
+ * `SafeERC20`: add `safePermit`. ([#3280](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3280))
  * `Math`: add a `mulDiv` function that can round the result either up or down. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
  * `Strings`: add a new overloaded function `toHexString` that converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation. ([#3403](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3403))
  * `EnumerableMap`: add new `UintToUintMap` map type. ([#3338](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3338))

From 2d21d7ce0f6db13cead53160ae0a3cc4b9d63ebe Mon Sep 17 00:00:00 2001
From: Francisco Giordano <frangio.1@gmail.com>
Date: Mon, 6 Jun 2022 18:13:05 -0300
Subject: [PATCH 06/11] move Permit type definition to single place

---
 test/helpers/eip712.js                                |  9 +++++++++
 test/token/ERC20/extensions/draft-ERC20Permit.test.js | 10 +---------
 test/token/ERC20/utils/SafeERC20.test.js              | 10 +---------
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/test/helpers/eip712.js b/test/helpers/eip712.js
index 4f8fe7dfb27..8a64b8a35bd 100644
--- a/test/helpers/eip712.js
+++ b/test/helpers/eip712.js
@@ -7,6 +7,14 @@ const EIP712Domain = [
   { name: 'verifyingContract', type: 'address' },
 ];
 
+const Permit = [
+  { name: 'owner', type: 'address' },
+  { name: 'spender', type: 'address' },
+  { name: 'value', type: 'uint256' },
+  { name: 'nonce', type: 'uint256' },
+  { name: 'deadline', type: 'uint256' },
+];
+
 async function domainSeparator (name, version, chainId, verifyingContract) {
   return '0x' + ethSigUtil.TypedDataUtils.hashStruct(
     'EIP712Domain',
@@ -17,5 +25,6 @@ async function domainSeparator (name, version, chainId, verifyingContract) {
 
 module.exports = {
   EIP712Domain,
+  Permit,
   domainSeparator,
 };
diff --git a/test/token/ERC20/extensions/draft-ERC20Permit.test.js b/test/token/ERC20/extensions/draft-ERC20Permit.test.js
index 9aa64456110..fb60a6cbe52 100644
--- a/test/token/ERC20/extensions/draft-ERC20Permit.test.js
+++ b/test/token/ERC20/extensions/draft-ERC20Permit.test.js
@@ -10,15 +10,7 @@ const Wallet = require('ethereumjs-wallet').default;
 
 const ERC20PermitMock = artifacts.require('ERC20PermitMock');
 
-const { EIP712Domain, domainSeparator } = require('../../../helpers/eip712');
-
-const Permit = [
-  { name: 'owner', type: 'address' },
-  { name: 'spender', type: 'address' },
-  { name: 'value', type: 'uint256' },
-  { name: 'nonce', type: 'uint256' },
-  { name: 'deadline', type: 'uint256' },
-];
+const { EIP712Domain, Permit, domainSeparator } = require('../../../helpers/eip712');
 
 contract('ERC20Permit', function (accounts) {
   const [ initialHolder, spender, recipient, other ] = accounts;
diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js
index 08934e6f206..aa7338cbd54 100644
--- a/test/token/ERC20/utils/SafeERC20.test.js
+++ b/test/token/ERC20/utils/SafeERC20.test.js
@@ -6,7 +6,7 @@ const ERC20NoReturnMock = artifacts.require('ERC20NoReturnMock');
 const ERC20PermitNoRevertMock = artifacts.require('ERC20PermitNoRevertMock');
 const SafeERC20Wrapper = artifacts.require('SafeERC20Wrapper');
 
-const { EIP712Domain } = require('../../../helpers/eip712');
+const { EIP712Domain, Permit } = require('../../../helpers/eip712');
 
 const { fromRpcSig } = require('ethereumjs-util');
 const ethSigUtil = require('eth-sig-util');
@@ -48,14 +48,6 @@ contract('SafeERC20', function (accounts) {
   });
 
   describe('with token that doesn\'t revert on invalid permit', function () {
-    const Permit = [
-      { name: 'owner', type: 'address' },
-      { name: 'spender', type: 'address' },
-      { name: 'value', type: 'uint256' },
-      { name: 'nonce', type: 'uint256' },
-      { name: 'deadline', type: 'uint256' },
-    ];
-
     const wallet = Wallet.generate();
     const owner = wallet.getAddressString();
     const spender = hasNoCode;

From 4fa970e7d3295e39387b1874089e6312684e0009 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Tue, 7 Jun 2022 08:28:13 +0200
Subject: [PATCH 07/11] Update CHANGELOG.md

Co-authored-by: Francisco <frangio.1@gmail.com>
---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7127ade2e6..f8f9d6f25a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,7 @@
  * `CrossChainEnabledPolygonChild`: replace the `require` statement with the custom error `NotCrossChainCall`. ([#3380](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3380))
  * `ERC20FlashMint`: Add customizable flash fee receiver. ([#3327](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3327))
  * `ERC20TokenizedVault`: add an extension of `ERC20` that implements the ERC4626 Tokenized Vault Standard. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
- * `SafeERC20`: add `safePermit`. ([#3280](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3280))
+ * `SafeERC20`: add `safePermit` as mitigation against phantom permit functions. ([#3280](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3280))
  * `Math`: add a `mulDiv` function that can round the result either up or down. ([#3171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3171))
  * `Strings`: add a new overloaded function `toHexString` that converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation. ([#3403](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3403))
  * `EnumerableMap`: add new `UintToUintMap` map type. ([#3338](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3338))

From fd7279849dbff4fabd910198bd42ac43d69b5653 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Tue, 7 Jun 2022 08:39:29 +0200
Subject: [PATCH 08/11] address comments for the PR

---
 contracts/mocks/SafeERC20Helper.sol                  | 12 ++++++------
 .../token/ERC20/extensions/draft-IERC20Permit.sol    |  2 +-
 contracts/token/ERC20/utils/SafeERC20.sol            |  5 +----
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol
index e159f61e2ea..af3420e3d17 100644
--- a/contracts/mocks/SafeERC20Helper.sol
+++ b/contracts/mocks/SafeERC20Helper.sol
@@ -114,7 +114,7 @@ contract ERC20PermitNoRevertMock is
         return block.chainid;
     }
 
-    function permitRevert(
+    function permitThatMayRevert(
         address owner,
         address spender,
         uint256 value,
@@ -135,7 +135,7 @@ contract ERC20PermitNoRevertMock is
         bytes32 r,
         bytes32 s
     ) public virtual override {
-        try this.permitRevert(owner, spender, value, deadline, v, r, s) {
+        try this.permitThatMayRevert(owner, spender, value, deadline, v, r, s) {
             // do nothing
         } catch {
             // do nothing
@@ -144,11 +144,11 @@ contract ERC20PermitNoRevertMock is
 }
 
 contract SafeERC20Wrapper is Context {
-    using SafeERC20 for IERC20Permit;
+    using SafeERC20 for IERC20;
 
-    IERC20Permit private _token;
+    IERC20 private _token;
 
-    constructor(IERC20Permit token) {
+    constructor(IERC20 token) {
         _token = token;
     }
 
@@ -181,7 +181,7 @@ contract SafeERC20Wrapper is Context {
         bytes32 r,
         bytes32 s
     ) public {
-        _token.safePermit(owner, spender, value, deadline, v, r, s);
+        SafeERC20.safePermit(IERC20Permit(address(_token)), owner, spender, value, deadline, v, r, s);
     }
 
     function setAllowance(uint256 allowance_) public {
diff --git a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
index 6572fddf6e4..e64f0260387 100644
--- a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
+++ b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
@@ -13,7 +13,7 @@ import "../IERC20.sol";
  * presenting a message signed by the account. By not relying on {IERC20-approve}, the token holder account doesn't
  * need to send a transaction, and thus is not required to hold Ether at all.
  */
-interface IERC20Permit is IERC20 {
+interface IERC20Permit {
     /**
      * @dev Sets `value` as the allowance of `spender` over ``owner``'s tokens,
      * given ``owner``'s signed approval.
diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol
index d87d080f9ee..31053ee7d42 100644
--- a/contracts/token/ERC20/utils/SafeERC20.sol
+++ b/contracts/token/ERC20/utils/SafeERC20.sol
@@ -91,10 +91,7 @@ library SafeERC20 {
         bytes32 s
     ) internal {
         uint256 nonceBefore = token.nonces(owner);
-        _callOptionalReturn(
-            token,
-            abi.encodeWithSelector(token.permit.selector, owner, spender, value, deadline, v, r, s)
-        );
+        token.permit(owner, spender, value, deadline, v, r, s);
         uint256 nonceAfter = token.nonces(owner);
         require(nonceAfter == nonceBefore + 1, "SafeERC20: permit did not succeed");
     }

From 048c2b6c2ed096fb3ee40c603716145441e175fd Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Tue, 7 Jun 2022 08:40:38 +0200
Subject: [PATCH 09/11] Update draft-IERC20Permit.sol

---
 contracts/token/ERC20/extensions/draft-IERC20Permit.sol | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
index e64f0260387..6363b14084e 100644
--- a/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
+++ b/contracts/token/ERC20/extensions/draft-IERC20Permit.sol
@@ -3,8 +3,6 @@
 
 pragma solidity ^0.8.0;
 
-import "../IERC20.sol";
-
 /**
  * @dev Interface of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in
  * https://eips.ethereum.org/EIPS/eip-2612[EIP-2612].

From f0b6fc48e4afb5420693e42cbc890264ec84ce77 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Tue, 7 Jun 2022 15:47:56 +0200
Subject: [PATCH 10/11] refactor SafeERC20 safePermit tests

---
 test/token/ERC20/utils/SafeERC20.test.js | 67 ++++++++++++------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js
index aa7338cbd54..980396ce822 100644
--- a/test/token/ERC20/utils/SafeERC20.test.js
+++ b/test/token/ERC20/utils/SafeERC20.test.js
@@ -87,7 +87,7 @@ contract('SafeERC20', function (accounts) {
 
     it('revert on reused signature', async function () {
       expect(await this.token.nonces(owner)).to.be.bignumber.equal('0');
-
+      // use valid signature and consume nounce
       await this.wrapper.permit(
         this.data.message.owner,
         this.data.message.spender,
@@ -97,9 +97,19 @@ contract('SafeERC20', function (accounts) {
         this.signature.r,
         this.signature.s,
       );
-
       expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
-
+      // invalid call does not revert for this token implementation
+      await this.token.permit(
+        this.data.message.owner,
+        this.data.message.spender,
+        this.data.message.value,
+        this.data.message.deadline,
+        this.signature.v,
+        this.signature.r,
+        this.signature.s,
+      );
+      expect(await this.token.nonces(owner)).to.be.bignumber.equal('1');
+      // invalid call revert when called through the SafeERC20 library
       await expectRevert(
         this.wrapper.permit(
           this.data.message.owner,
@@ -116,30 +126,14 @@ contract('SafeERC20', function (accounts) {
     });
 
     it('revert on invalid signature', async function () {
-      await expectRevert(
-        this.wrapper.permit(
-          this.data.message.owner,
-          this.data.message.spender,
-          this.data.message.value,
-          this.data.message.deadline,
-          27,
-          '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
-          '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
-        ),
-        'SafeERC20: permit did not succeed',
-      );
-    });
+      // signature that is not valid for owner
+      this.signature = {
+        v: 27,
+        r: '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
+        s: '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
+      };
 
-    it('underlying token does not revert on reused or invalid signature', async function () {
-      await this.token.permit(
-        this.data.message.owner,
-        this.data.message.spender,
-        this.data.message.value,
-        this.data.message.deadline,
-        this.signature.v,
-        this.signature.r,
-        this.signature.s,
-      );
+      // invalid call does not revert for this token implementation
       await this.token.permit(
         this.data.message.owner,
         this.data.message.spender,
@@ -149,14 +143,19 @@ contract('SafeERC20', function (accounts) {
         this.signature.r,
         this.signature.s,
       );
-      await this.token.permit(
-        this.data.message.owner,
-        this.data.message.spender,
-        this.data.message.value,
-        this.data.message.deadline,
-        27,
-        '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
-        '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
+
+      // invalid call revert when called through the SafeERC20 library
+      await expectRevert(
+        this.wrapper.permit(
+          this.data.message.owner,
+          this.data.message.spender,
+          this.data.message.value,
+          this.data.message.deadline,
+          this.signature.v,
+          this.signature.r,
+          this.signature.s,
+        ),
+        'SafeERC20: permit did not succeed',
       );
     });
   });

From d0ab2dce775b38cf96b3ea19e19364e078f491b8 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <frangio.1@gmail.com>
Date: Tue, 7 Jun 2022 10:53:34 -0300
Subject: [PATCH 11/11] use different variable for signature

---
 test/token/ERC20/utils/SafeERC20.test.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/token/ERC20/utils/SafeERC20.test.js b/test/token/ERC20/utils/SafeERC20.test.js
index 980396ce822..4c54c583059 100644
--- a/test/token/ERC20/utils/SafeERC20.test.js
+++ b/test/token/ERC20/utils/SafeERC20.test.js
@@ -127,7 +127,7 @@ contract('SafeERC20', function (accounts) {
 
     it('revert on invalid signature', async function () {
       // signature that is not valid for owner
-      this.signature = {
+      const invalidSignature = {
         v: 27,
         r: '0x71753dc5ecb5b4bfc0e3bc530d79ce5988760ed3f3a234c86a5546491f540775',
         s: '0x0049cedee5aed990aabed5ad6a9f6e3c565b63379894b5fa8b512eb2b79e485d',
@@ -139,9 +139,9 @@ contract('SafeERC20', function (accounts) {
         this.data.message.spender,
         this.data.message.value,
         this.data.message.deadline,
-        this.signature.v,
-        this.signature.r,
-        this.signature.s,
+        invalidSignature.v,
+        invalidSignature.r,
+        invalidSignature.s,
       );
 
       // invalid call revert when called through the SafeERC20 library
@@ -151,9 +151,9 @@ contract('SafeERC20', function (accounts) {
           this.data.message.spender,
           this.data.message.value,
           this.data.message.deadline,
-          this.signature.v,
-          this.signature.r,
-          this.signature.s,
+          invalidSignature.v,
+          invalidSignature.r,
+          invalidSignature.s,
         ),
         'SafeERC20: permit did not succeed',
       );