Merge branch 'main' of https://github.com/safe-global/safe-contracts into test-checknsigs

Author: mmv08

contracts/Safe.sol

@@ -257,20 +257,7 @@ contract Safe is
         uint256 _threshold = threshold;
         // Check that a threshold is set
         require(_threshold > 0, "GS001");
-        checkNSignatures(dataHash, data, signatures, _threshold);
-    }
-
-    /**
-     * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
-     * @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
-     * @param dataHash Hash of the data (could be either a message hash or transaction hash)
-     * @param data That should be signed (this is passed to an external validator contract)
-     * @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.
-     * @param requiredSignatures Amount of required valid signatures.
-     */
-    function checkNSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
-        return checkNSignatures(msg.sender, dataHash, data, signatures, requiredSignatures);
+        checkNSignatures(msg.sender, dataHash, data, signatures, _threshold);
     }
 
     /**

contracts/handler/CompatibilityFallbackHandler.sol

@@ -4,12 +4,13 @@ pragma solidity >=0.7.0 <0.9.0;
 import "./TokenCallbackHandler.sol";
 import "../interfaces/ISignatureValidator.sol";
 import "../Safe.sol";
+import "./HandlerContext.sol";
 
 /**
  * @title Compatibility Fallback Handler - Provides compatibility between pre 1.3.0 and 1.3.0+ Safe contracts.
  * @author Richard Meissner - @rmeissner
  */
-contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidator {
+contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidator, HandlerContext {
     // keccak256("SafeMessage(bytes message)");
     bytes32 private constant SAFE_MSG_TYPEHASH = 0x60b3cbf8b4a223d68d641b3b6ddf9a298e7f33710cf3d3a9d1146b5a6150fbca;
 
@@ -154,4 +155,19 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
             }
         }
     }
+
+    /**
+     * @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
+     * @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
+     *      The function was moved to the fallback handler as a part of
+     *      1.5.0 contract upgrade. It used to be a part of the Safe core contract, but
+     *      was replaced by the same function that accepts the executor as a parameter.
+     * @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.
+     * @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);
+    }
 }

contracts/test/4337/Test4337ModuleAndHandler.sol

@@ -2,8 +2,6 @@
 pragma solidity >=0.7.0 <0.9.0;
 pragma abicoder v2;
 
-import "../../libraries/SafeStorage.sol";
-
 struct UserOperation {
     address sender;
     uint256 nonce;
@@ -19,14 +17,16 @@ struct UserOperation {
 }
 
 interface ISafe {
+    function enableModule(address module) external;
+
     function execTransactionFromModule(address to, uint256 value, bytes memory data, uint8 operation) external returns (bool success);
 }
 
 /// @dev A Dummy 4337 Module/Handler for testing purposes
 ///      ⚠️ ⚠️ ⚠️ DO NOT USE IN PRODUCTION ⚠️ ⚠️ ⚠️
 ///      The module does not perform ANY validation, it just executes validateUserOp and execTransaction
 ///      to perform the opcode level compliance by the bundler.
-contract Test4337ModuleAndHandler is SafeStorage {
+contract Test4337ModuleAndHandler {
     address public immutable myAddress;
     address public immutable entryPoint;
 
@@ -55,11 +55,6 @@ contract Test4337ModuleAndHandler is SafeStorage {
     }
 
     function enableMyself() public {
-        require(myAddress != address(this), "You need to DELEGATECALL, sir");
-
-        // Module cannot be added twice.
-        require(modules[myAddress] == address(0), "GS102");
-        modules[myAddress] = modules[SENTINEL_MODULES];
-        modules[SENTINEL_MODULES] = myAddress;
+        ISafe(address(this)).enableModule(myAddress);
     }
 }

docs/safe_tx_gas.md

@@ -26,7 +26,7 @@ This also results in the `nonce` of this transaction being used, so it is not po
 
 #### Without gas refund
 
-If `gasPrice` is set to `0` then the Safe contracts will **not** issue a refund after the Safe transaction exection.
+If `gasPrice` is set to `0` then the Safe contracts will **not** issue a refund after the Safe transaction execution.
 
 Therefore it is not necessary to be as strict on the gas being passed along with the execution of the Safe transaction. As no refund is triggered the Safe will not pay for the execution costs, based on this the Safe contracts will send along all available case when no refund is used.