Reduce ERC20 allowance before triggering transfer

CHANGELOG.md

@@ -11,6 +11,7 @@
  * `Votes`: Added a base contract for vote tracking with delegation. ([#2944](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2944))
  * `ERC721Votes`: Added an extension of ERC721 enabled with vote tracking and delegation. ([#2944](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2944))
  * `ERC2771Context`: use immutable storage to store the forwarder address, no longer an issue since Solidity >=0.8.8 allows reading immutable variables in the constructor. ([#2917](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2917))
+ * `ERC20`: reduce allowance before triggering transfer.
 
 ## 4.4.1 (2021-12-14)
 

contracts/token/ERC20/ERC20.sol

@@ -152,14 +152,14 @@ contract ERC20 is Context, IERC20, IERC20Metadata {
         address recipient,
         uint256 amount
     ) public virtual override returns (bool) {
-        _transfer(sender, recipient, amount);
-
         uint256 currentAllowance = _allowances[sender][_msgSender()];
         require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance");
         unchecked {
             _approve(sender, _msgSender(), currentAllowance - amount);
         }
 
+        _transfer(sender, recipient, amount);
+
         return true;
     }
 

contracts/token/ERC777/ERC777.sol

@@ -287,12 +287,12 @@ contract ERC777 is Context, IERC777, IERC20 {
 
         _callTokensToSend(spender, holder, recipient, amount, "", "");
 
-        _move(spender, holder, recipient, amount, "", "");
-
         uint256 currentAllowance = _allowances[holder][spender];
         require(currentAllowance >= amount, "ERC777: transfer amount exceeds allowance");
         _approve(holder, spender, currentAllowance - amount);
 
+        _move(spender, holder, recipient, amount, "", "");
+
         _callTokensReceived(spender, holder, recipient, amount, "", "", false);
 
         return true;

test/token/ERC20/ERC20.behavior.js

@@ -40,7 +40,7 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
       describe('when the recipient is not the zero address', function () {
         const to = anotherAccount;
 
-        describe('when the spender has enough approved balance', function () {
+        describe('when the spender has enough approved allowance', function () {
           beforeEach(async function () {
             await this.token.approve(spender, initialSupply, { from: initialHolder });
           });
@@ -84,37 +84,50 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
           });
 
           describe('when the token owner does not have enough balance', function () {
-            const amount = initialSupply.addn(1);
+            const amount = initialSupply;
+
+            beforeEach('reducing balance', async function () {
+              await this.token.transfer(to, 1, { from: tokenOwner });
+            });
 
             it('reverts', async function () {
-              await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`,
+              await expectRevert(
+                this.token.transferFrom(tokenOwner, to, amount, { from: spender }),
+                `${errorPrefix}: transfer amount exceeds balance`,
               );
             });
           });
         });
 
-        describe('when the spender does not have enough approved balance', function () {
+        describe('when the spender does not have enough approved allowance', function () {
+          const allowance = initialSupply.subn(1);
+
           beforeEach(async function () {
-            await this.token.approve(spender, initialSupply.subn(1), { from: tokenOwner });
+            await this.token.approve(spender, allowance, { from: tokenOwner });
           });
 
           describe('when the token owner has enough balance', function () {
             const amount = initialSupply;
 
             it('reverts', async function () {
-              await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds allowance`,
+              await expectRevert(
+                this.token.transferFrom(tokenOwner, to, amount, { from: spender }),
+                `${errorPrefix}: transfer amount exceeds allowance`,
               );
             });
           });
 
           describe('when the token owner does not have enough balance', function () {
-            const amount = initialSupply.addn(1);
+            const amount = allowance;
+
+            beforeEach('reducing balance', async function () {
+              await this.token.transfer(to, initialSupply, { from: tokenOwner });
+            });
 
             it('reverts', async function () {
-              await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`,
+              await expectRevert(
+                this.token.transferFrom(tokenOwner, to, amount, { from: spender }),
+                `${errorPrefix}: transfer amount exceeds balance`,
               );
             });
           });
@@ -143,8 +156,9 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
       const to = recipient;
 
       it('reverts', async function () {
-        await expectRevert(this.token.transferFrom(
-          tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer from the zero address`,
+        await expectRevert(
+          this.token.transferFrom(tokenOwner, to, amount, { from: spender }),
+          `from the zero address`,
         );
       });
     });