Gas Optimization: consider removing redundant check in PermitERC721

[naszam]

Context: PermitERC721.sol#L108

Description:
PermitERC721 currently uses a check to ensure recoveredAddress != address(0) even though it should be covered by the check recoveredAddress == owner, where owner is already checked to never been equal to address(0) in ownerOf function of ERC721 from openzeppelin-contracts.

Recommendation:
Consider removing the check to improve gas efficiency.
Ensure openzeppelin-contracts implementation of ownerOf doesn't change, otherwise this will break the contract (see here).

diff --git a/src/base/PermitERC721.sol b/src/base/PermitERC721.sol
index a92acaf..ece506a 100644
--- a/src/base/PermitERC721.sol
+++ b/src/base/PermitERC721.sol
@@ -105,7 +105,6 @@ abstract contract PermitERC721 is ERC721, IPermit {
             );
         } else {
             address recoveredAddress = ecrecover(digest, v_, r_, s_);
-            require(recoveredAddress != address(0), "ajna/nft-invalid-signature");
             require(recoveredAddress == owner, "ajna/nft-unauthorized");
         }

diff --git a/tests/forge/unit/PositionManager.t.sol b/tests/forge/unit/PositionManager.t.sol
index bf3aa40..160b9d2 100644
--- a/tests/forge/unit/PositionManager.t.sol
+++ b/tests/forge/unit/PositionManager.t.sol
@@ -1386,7 +1386,7 @@ contract PositionManagerERC20PoolTest is PositionManagerERC20PoolHelperContract
         // check signature is valid
         deadline = block.timestamp + 1 days;
         (v, r, s) = _getPermitSig(testSpender, tokenId, deadline, minterPrivateKey);
-        vm.expectRevert("ajna/nft-invalid-signature");
+        vm.expectRevert("ajna/nft-unauthorized");
         spenderContract.transferFromWithPermit(testReceiver, tokenId, deadline, 0, r, s );
 
         // check can't self approve

• Tests PASS
• Gas Report:

Before

| src/PositionManager.sol:PositionManager contract |                 |        |        |         |         |
|--------------------------------------------------|-----------------|--------|--------|---------|---------|
| Deployment Cost                                  | Deployment Size |        |        |         |         |
| 3922538                                          | 19868           |        |        |         |         |
| Function Name                                    | min             | avg    | median | max     | # calls |
| permit                                           | 618             | 30939  | 27698  | 54387   | 7       |

After

| src/PositionManager.sol:PositionManager contract |                 |        |        |         |         |
|--------------------------------------------------|-----------------|--------|--------|---------|---------|
| Deployment Cost                                  | Deployment Size |        |        |         |         |
| 3905113                                          | 19781           |        |        |         |         |
| Function Name                                    | min             | avg    | median | max     | # calls |
| permit                                           | 618             | 32060  | 28843  | 54387   | 6       |

Ajna:
PermitERC721 have been refactored to follow standard reference implementation (see here), where the explicit zero address check is now required due to extra conditions in isApprovedOrOwner function.

Prototech: