Remove bytes from core contract methods and add them to the compatibi…

…lity fallback handler
safe-global · Oct 31, 2023 · f34eaf9 · f34eaf9
1 parent b344bd8
commit f34eaf9
Show file tree

Hide file tree

Showing 3 changed files with 210 additions and 43 deletions.
diff --git a/contracts/Safe.sol b/contracts/Safe.sol
@@ -166,7 +166,7 @@ contract Safe is
                 // We use the post-increment here, so the current nonce value is used and incremented afterwards.
                 nonce++
             );
-            checkSignatures(txHash, "", signatures);
+            checkSignatures(txHash, signatures);
         }
         address guard = getGuard();
         {
@@ -285,17 +285,15 @@ contract Safe is
     /**
      * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
      * @param dataHash Hash of the data (could be either a message hash or transaction hash)
-     * @param data NOT USED. That should be signed (this is passed to an external validator contract)
-     *             For some reason, removing it from the parameters and passing empty bytes ("") slightly increases the gas costs, so we keep it.
      * @param signatures Signature data that should be verified.
      *                   Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
      */
-    function checkSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures) public view {
+    function checkSignatures(bytes32 dataHash, bytes memory signatures) public view {
         // Load threshold to avoid multiple storage loads
         uint256 _threshold = threshold;
         // Check that a threshold is set
         require(_threshold > 0, "GS001");
-        checkNSignatures(msg.sender, dataHash, data, signatures, _threshold);
+        checkNSignatures(msg.sender, dataHash, signatures, _threshold);
     }
 
     /**
@@ -312,13 +310,7 @@ contract Safe is
      *                   Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
      * @param requiredSignatures Amount of required valid signatures.
      */
-    function checkNSignatures(
-        address executor,
-        bytes32 dataHash,
-        bytes memory /* data */,
-        bytes memory signatures,
-        uint256 requiredSignatures
-    ) public view {
+    function checkNSignatures(address executor, bytes32 dataHash, bytes memory signatures, uint256 requiredSignatures) public view {
         // Check that the provided signature data is not too short
         require(signatures.length >= requiredSignatures.mul(65), "GS020");
         // There cannot be an owner with address 0.

diff --git a/contracts/handler/CompatibilityFallbackHandler.sol b/contracts/handler/CompatibilityFallbackHandler.sol
@@ -62,7 +62,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
         if (_signature.length == 0) {
             require(safe.signedMessages(messageHash) != 0, "Hash not approved");
         } else {
-            safe.checkSignatures(messageHash, messageData, _signature);
+            safe.checkSignatures(messageHash, _signature);
         }
         return EIP1271_MAGIC_VALUE;
     }
@@ -169,6 +169,19 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
      * @param requiredSignatures Amount of required valid signatures.
      */
     function checkNSignatures(bytes32 dataHash, bytes memory, bytes memory signatures, uint256 requiredSignatures) public view {
-        Safe(payable(_manager())).checkNSignatures(_msgSender(), dataHash, "", signatures, requiredSignatures);
+        Safe(payable(_manager())).checkNSignatures(_msgSender(), dataHash, signatures, requiredSignatures);
+    }
+
+    /**
+     * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
+     * @param dataHash Hash of the data (could be either a message hash or transaction hash)
+     * @param signatures Signature data that should be verified.
+     *                   Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
+     */
+    function checkSignatures(bytes32 dataHash, bytes memory, bytes memory signatures) public view {
+        Safe safe = Safe(payable(_manager()));
+
+        uint256 threshold = safe.getThreshold();
+        safe.checkNSignatures(_msgSender(), dataHash, signatures, threshold);
     }
 }