Revert "Increment nonce on approve to avoid signature replay (#906)" (#…

…920) This reverts commit d4c9ad2.
ajna-finance · Jul 3, 2023 · f61fba5 · f61fba5
1 parent 2af7d81
commit f61fba5
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 39 deletions.
diff --git a/src/base/PermitERC721.sol b/src/base/PermitERC721.sol
@@ -278,21 +278,6 @@ abstract contract PermitERC721 is ERC721, IPermit {
         _nonces[tokenId]++;
     }
 
-    /**
-     * @notice _approve override to be able to increment the permit nonce
-     * @inheritdoc ERC721
-     */
-    function _approve(
-        address to,
-        uint256 tokenId
-    ) internal virtual override {
-        // increment the permit nonce of this tokenId to ensure it can't be reused
-        _incrementNonce(tokenId);
-
-        // Approve the NFT to the to address
-        super._approve(to, tokenId);
-    }
-
     /**
      * @notice _transfer override to be able to increment the permit nonce
      * @inheritdoc ERC721

diff --git a/tests/forge/unit/Positions/PositionManager.t.sol b/tests/forge/unit/Positions/PositionManager.t.sol
@@ -1330,7 +1330,7 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
         spenderContract.transferFromWithPermit(address(recipientContract), tokenId, deadline, signature);
 
         // check nonces increment with transfer
-        assertEq(_positionManager.nonces(tokenId), 2);
+        assertEq(_positionManager.nonces(tokenId), 1);
 
         // check retrieving token nonces for non existent tokens will revert
         vm.expectRevert(IPermit.NonExistentToken.selector);
@@ -1379,28 +1379,6 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
         assertEq(_positionManager.nonces(tokenId), 0);
     }
 
-    function testPermitSignatureReplayReverts() external {
-        // generate addresses and set test params
-        (address testMinter, uint256 minterPrivateKey) = makeAddrAndKey("testMinter");
-        address testSpender = makeAddr("spender");
-
-        changePrank(testMinter);
-        uint256 tokenId = _mintNFT(testMinter, testMinter, address(_pool));
-        assertEq(_positionManager.ownerOf(tokenId), testMinter);
-
-        uint256 deadline = block.timestamp + 1 days;
-        bytes memory signature = _getPermitSig(testSpender, tokenId, 0, deadline, minterPrivateKey);
-
-        _positionManager.permit(testSpender, tokenId, deadline, signature);
-
-        // minter revokes approval
-        _positionManager.approve(address(0), tokenId);
-
-        // spender tries to get approval with the same signature
-        vm.expectRevert(IPermit.NotAuthorized.selector);
-        _positionManager.permit(testSpender, tokenId, deadline, signature);
-    }
-
     /**
      *  @notice Tests permit signatures are invalid after each transfer due to incremented nonce.
      */
@@ -1464,7 +1442,7 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
         assertEq(_positionManager.ownerOf(tokenId), testMinter);
 
         // check nonces after transfer
-        assertEq(_positionManager.nonces(tokenId), 3);
+        assertEq(_positionManager.nonces(tokenId), 2);
     }
 
     /**