Merge pull request #198 from bcnmy/refactor/pr-196-finding-44

refactor: respond to PR comment by 0xbok
bcnmy · Oct 7, 2024 · c4c3a0f · c4c3a0f
2 parents a9eee7b + d312888
commit c4c3a0f
Showing 1 changed file with 12 additions and 32 deletions.
diff --git a/contracts/modules/validators/K1Validator.sol b/contracts/modules/validators/K1Validator.sol
@@ -171,22 +171,6 @@ contract K1Validator is IValidator, ERC7739Validator {
         return _validateSignatureForOwner(owner, hash, sig);
     }
 
-    /// @notice Recovers the signer from a signature
-    /// @param hash The hash of the data to validate
-    /// @param signature The signature data
-    /// @return The recovered signer address
-    function recoverSigner(bytes32 hash, bytes calldata signature) external view returns (address) {
-        return hash.recover(signature);
-    }
-
-    /// @notice Recovers the signer from an Ethereum signed message
-    /// @param hash The hash of the data to validate
-    /// @param signature The signature data
-    /// @return The recovered signer address
-    function recoverSignerFromEthSignedMessage(bytes32 hash, bytes calldata signature) external view returns (address) {
-        return hash.toEthSignedMessageHash().recover(signature);
-    }
-
     /*//////////////////////////////////////////////////////////////////////////
                                      METADATA
     //////////////////////////////////////////////////////////////////////////*/
@@ -214,6 +198,15 @@ contract K1Validator is IValidator, ERC7739Validator {
                                      INTERNAL
     //////////////////////////////////////////////////////////////////////////*/
 
+    /// @notice Recovers the signer from a signature
+    /// @param hash The hash of the data to validate
+    /// @param signature The signature data
+    /// @return The recovered signer address
+    /// @notice tryRecover returns address(0) on invalid signature
+    function _recoverSigner(bytes32 hash, bytes calldata signature) internal view returns (address) {
+        return hash.tryRecover(signature);
+    }
+
     /// @dev Returns whether the `hash` and `signature` are valid.
     ///      Obtains the authorized signer's credentials and calls some
     ///      module's specific internal function to validate the signature
@@ -252,22 +245,9 @@ contract K1Validator is IValidator, ERC7739Validator {
         }
 
         // verify signer
-        try this.recoverSigner(hash, signature) returns (address recoveredSigner) {
-            if (recoveredSigner == owner) {
-                return true;
-            }
-        } catch {
-            // If recovery fails, we'll continue to the next check
-        }
-
-        try this.recoverSignerFromEthSignedMessage(hash, signature) returns (address recoveredSigner) {
-            if (recoveredSigner == owner) {
-                return true;
-            }
-        } catch {
-            // If recovery fails, we'll return false
-        }
-
+        // owner can not be zero address in this contract
+        if (_recoverSigner(hash, signature) == owner) return true;
+        if (_recoverSigner(hash.toEthSignedMessageHash(), signature) == owner) return true;
         return false;
     }