Merge branch 'main' of github.com:safe-global/safe-contracts into add…

…_tests_eip1271

.env.sample

@@ -4,5 +4,3 @@ INFURA_KEY=""
 # Used for custom network
 NODE_URL=""
 ETHERSCAN_API_KEY=""
-# Use the Safe singleton factory for singleton deployment. This is required if EIP-155 is enforce on a chain.
-# CUSTOM_DETERMINISTIC_DEPLOYMENT="true"

.github/ISSUE_TEMPLATE/bug_report.md

@@ -7,7 +7,7 @@ about: Bug report about the Safe smart contracts
 ## Prerequisites
 
 - First, many thanks for taking part in the community and helping us improve. We appreciate that a lot.
-- Support questions are better asked in our Gitter chat: https://gitter.im/gnosis/Safe
+- Support questions are better asked in our Discord: https://chat.safe.global
 - Please ensure the issue isn't already reported.
 
 *Please delete the above section and the instructions in the sections below before submitting*

.github/ISSUE_TEMPLATE/feature_request.md

@@ -7,7 +7,7 @@ about: Suggest an idea or feature request for the Safe smart contracts project
 ## Prerequisites
 
 - First, many thanks for taking part in the community and helping us improve. We appreciate that a lot.
-- Support questions are better asked in our Gitter chat: https://gitter.im/gnosis/Safe
+- Support questions are better asked in our Discord: https://chat.safe.global
 - Please ensure the issue isn't already reported.
 
 *Please delete the above section and the instructions in the sections below before submitting*

.github/workflows/ci.yml

@@ -38,7 +38,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        contract-name: ["GnosisSafeL2"]
+        contract-name: ["SafeL2"]
     env:
       SAFE_CONTRACT_UNDER_TEST: ${{ matrix.contract-name }}
     steps:
@@ -78,4 +78,4 @@ jobs:
         with:
           path: "**/node_modules"
           key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
-      - run: (yarn --frozen-lockfile && yarn build && yarn hardhat codesize --contractname GnosisSafe && yarn benchmark) || echo "Benchmark failed"
+      - run: (yarn --frozen-lockfile && yarn build && yarn hardhat codesize --contractname Safe && yarn benchmark) || echo "Benchmark failed"

README.md

@@ -1,11 +1,11 @@
-Gnosis Safe Contracts
-=====================
+Safe Contracts
+==============
 
 [![npm version](https://badge.fury.io/js/%40gnosis.pm%2Fsafe-contracts.svg)](https://badge.fury.io/js/%40gnosis.pm%2Fsafe-contracts)
-[![Build Status](https://github.com/gnosis/safe-contracts/workflows/safe-contracts/badge.svg?branch=development)](https://github.com/gnosis/safe-contracts/actions)
-[![Coverage Status](https://coveralls.io/repos/github/gnosis/safe-contracts/badge.svg?branch=development)](https://coveralls.io/github/gnosis/safe-contracts)
+[![Build Status](https://github.com/safe-global/safe-contracts/workflows/safe-contracts/badge.svg?branch=development)](https://github.com/safe-global/safe-contracts/actions)
+[![Coverage Status](https://coveralls.io/repos/github/safe-global/safe-contracts/badge.svg?branch=development)](https://coveralls.io/github/safe-global/safe-contracts)
 
-> :warning: **This branch contains changes that are under development** To use the latest audited version make sure to use the correct commit. The tagged versions that are used by the Gnosis Safe team can be found in the [releases](https://github.com/gnosis/safe-contracts/releases).
+> :warning: **This branch contains changes that are under development** To use the latest audited version make sure to use the correct commit. The tagged versions that are used by the Safe team can be found in the [releases](https://github.com/safe-global/safe-contracts/releases).
 
 Usage
 -----
@@ -24,15 +24,15 @@ yarn test
 
 ### Deployments
 
-A collection of the different Safe contract deployments and their addresses can be found in the [Safe deployments](https://github.com/gnosis/safe-deployments) repository.
+A collection of the different Safe contract deployments and their addresses can be found in the [Safe deployments](https://github.com/safe-global/safe-deployments) repository.
 
-To add support for a new network follow the steps of the ``Deploy`` section and create a PR in the [Safe deployments](https://github.com/gnosis/safe-deployments) repository. 
+To add support for a new network follow the steps of the ``Deploy`` section and create a PR in the [Safe deployments](https://github.com/safe-global/safe-deployments) repository. 
 
 ### Deploy
 
-> :warning: **Make sure to use the correct commit when deploying the contracts.** Any change (even comments) within the contract files will result in different addresses. The tagged versions that are used by the Gnosis Safe team can be found in the [releases](https://github.com/gnosis/safe-contracts/releases).
+> :warning: **Make sure to use the correct commit when deploying the contracts.** Any change (even comments) within the contract files will result in different addresses. The tagged versions that are used by the Safe team can be found in the [releases](https://github.com/safe-global/safe-contracts/releases).
 
-> **Current version:** The latest release is [v1.3.0-libs.0](https://github.com/gnosis/safe-contracts/tree/v1.3.0-libs.0) on the commit [767ef36](https://github.com/gnosis/safe-contracts/commit/767ef36bba88bdbc0c9fe3708a4290cabef4c376)
+> **Current version:** The latest release is [v1.3.0-libs.0](https://github.com/safe-global/safe-contracts/tree/v1.3.0-libs.0) on the commit [767ef36](https://github.com/safe-global/safe-contracts/commit/767ef36bba88bdbc0c9fe3708a4290cabef4c376)
 
 This will deploy the contracts deterministically and verify the contracts on etherscan using [Solidity 0.7.6](https://github.com/ethereum/solidity/releases/tag/v0.7.6) by default.
 
@@ -66,11 +66,11 @@ Note: Address will vary if contract code is changed or a different Solidity vers
 
 #### Replay protection (EIP-155)
 
-Some networks require replay protection. This is not possible with the default deployment process as it relies on a presigned transaction without replay protection (see https://github.com/Arachnid/deterministic-deployment-proxy). 
+Some networks require replay protection, making it incompatible with the default deployment process as it relies on a presigned transaction without replay protection (see https://github.com/Arachnid/deterministic-deployment-proxy). 
 
-It is possible to enable deployment via a different determinisitic deployment proxy (https://github.com/gnosis/safe-singleton-factory). To enable this the `CUSTOM_DETERMINISTIC_DEPLOYMENT` env var has to be set to `true` (see `.env.sample`). To make sure that the latest version of this package is install, make sure to run `yarn add @gnosis.pm/safe-singleton-factory` before deployment.
+Safe contracts use a different deterministic deployment proxy (https://github.com/safe-global/safe-singleton-factory). To make sure that the latest version of this package is installed, make sure to run `yarn add @gnosis.pm/safe-singleton-factory` before deployment. For more information, including how to deploy the factory to a new network, please refer to the factory repo.  
 
-Note: This will result in different addresses compared to the default deployment process.
+Note: This will result in different addresses compared to hardhat's default deterministic deployment process.
 
 ### Verify contract
 
@@ -86,7 +86,7 @@ yarn hardhat --network <network> etherscan-verify
 
 Documentation
 -------------
-- [Safe developer portal](http://docs.gnosis.io/safe)
+- [Safe developer portal](http://docs.safe.global)
 - [Error codes](docs/error_codes.md)
 - [Coding guidelines](docs/guidelines.md)
 

benchmark/GnosisSafe.Creation.spec.ts → benchmark/Safe.Creation.spec.ts

@@ -2,7 +2,7 @@ import "@nomiclabs/hardhat-ethers";
 import { setupBenchmarkContracts } from "./utils/setup"
 
 const contractSetup = setupBenchmarkContracts(undefined, true)
-describe("GnosisSafe", async () => {
+describe("Safe", async () => {
     it("creation", async () => {
         await contractSetup()
     })

benchmark/Safe.Proxy.spec.ts

@@ -0,0 +1,34 @@
+import "@nomiclabs/hardhat-ethers";
+import { buildSafeTransaction } from "../src/utils/execution";
+import { benchmark } from "./utils/setup"
+import { getFactory } from "../test/utils/setup";
+
+benchmark("Proxy", [{
+    name: "creation",
+    prepare: async (contracts,_,nonce) => {
+        const factory = contracts.additions.factory
+        // We're cheating and passing the factory address as a singleton address to bypass a check that singleton contract exists
+        const data = factory.interface.encodeFunctionData("createProxyWithNonce", [factory.address, "0x", 0])
+        return buildSafeTransaction({ to: factory.address, data, safeTxGas: 1000000, nonce })
+    },
+    fixture: async () => {
+        return {
+            factory: await getFactory(),
+        }
+    }
+}])
+
+benchmark("Proxy", [{
+    name: "chain specific creation",
+    prepare: async (contracts,_,nonce) => {
+        const factory = contracts.additions.factory
+        // We're cheating and passing the factory address as a singleton address to bypass a check that singleton contract exists
+        const data = factory.interface.encodeFunctionData("createChainSpecificProxyWithNonce", [factory.address, "0x", 0])
+        return buildSafeTransaction({ to: factory.address, data, safeTxGas: 1000000, nonce })
+    },
+    fixture: async () => {
+        return {
+            factory: await getFactory(),
+        }
+    }
+}])

benchmark/utils/setup.ts

@@ -23,7 +23,7 @@ const generateTarget = async (owners: Wallet[], threshold: number, guardAddress:
 export const configs = [
     { name: "single owner", signers: [user1], threshold: 1 },
     { name: "single owner and guard", signers: [user1], threshold: 1, useGuard: true },
-    { name: "2 out of 23", signers: [user1, user2], threshold: 2 },
+    { name: "2 out of 2", signers: [user1, user2], threshold: 2 },
     { name: "3 out of 3", signers: [user1, user2, user3], threshold: 3 },
     { name: "3 out of 5", signers: [user1, user2, user3, user4, user5], threshold: 3 },
 ]
@@ -33,7 +33,7 @@ export const setupBenchmarkContracts = (benchmarkFixture?: () => Promise<any>, l
         await deployments.fixture();
         const guardFactory = await hre.ethers.getContractFactory("DelegateCallTransactionGuard");
         const guard = await guardFactory.deploy(AddressZero)
-        const targets = []
+        const targets: Contract[] = []
         for (const config of configs) {
             targets.push(await generateTarget(config.signers, config.threshold, config.useGuard ? guard.address : AddressZero, logGasUsage))
         }

contracts/GnosisSafe.sol → contracts/Safe.sol

@@ -11,12 +11,12 @@ import "./common/SignatureDecoder.sol";
 import "./common/SecuredTokenTransfer.sol";
 import "./common/StorageAccessible.sol";
 import "./interfaces/ISignatureValidator.sol";
-import "./external/GnosisSafeMath.sol";
+import "./external/SafeMath.sol";
 
-/// @title Gnosis Safe - A multisignature wallet with support for confirmations using signed messages based on ERC191.
+/// @title Safe - A multisignature wallet with support for confirmations using signed messages based on ERC191.
 /// @author Stefan George - <stefan@gnosis.io>
 /// @author Richard Meissner - <richard@gnosis.io>
-contract GnosisSafe is
+contract Safe is
     EtherPaymentFallback,
     Singleton,
     ModuleManager,
@@ -28,7 +28,7 @@ contract GnosisSafe is
     StorageAccessible,
     GuardManager
 {
-    using GnosisSafeMath for uint256;
+    using SafeMath for uint256;
 
     string public constant VERSION = "1.3.0";
 
@@ -255,6 +255,7 @@ contract GnosisSafe is
         for (i = 0; i < requiredSignatures; i++) {
             (v, r, s) = signatureSplit(signatures, i);
             if (v == 0) {
+                require(keccak256(data) == dataHash, "GS027");
                 // If v is 0 then it is a contract signature
                 // When handling contract signatures the address of the contract is encoded into r
                 currentOwner = address(uint160(uint256(r)));
@@ -303,29 +304,6 @@ contract GnosisSafe is
         }
     }
 
-    /// @dev Allows to estimate a Safe transaction.
-    ///      This method is only meant for estimation purpose, therefore the call will always revert and encode the result in the revert data.
-    ///      Since the `estimateGas` function includes refunds, call this method to get an estimated of the costs that are deducted from the safe with `execTransaction`
-    /// @param to Destination address of Safe transaction.
-    /// @param value Ether value of Safe transaction.
-    /// @param data Data payload of Safe transaction.
-    /// @param operation Operation type of Safe transaction.
-    /// @return Estimate without refunds and overhead fees (base transaction and payload data gas costs).
-    /// @notice Deprecated in favor of common/StorageAccessible.sol and will be removed in next version.
-    function requiredTxGas(
-        address to,
-        uint256 value,
-        bytes calldata data,
-        Enum.Operation operation
-    ) external returns (uint256) {
-        uint256 startGas = gasleft();
-        // We don't provide an error message here, as we use it to return the estimate
-        require(execute(to, value, data, operation, gasleft()));
-        uint256 requiredGas = startGas - gasleft();
-        // Convert response to string and return via error message
-        revert(string(abi.encodePacked(requiredGas)));
-    }
-
     /**
      * @dev Marks a hash as approved. This can be used to validate a hash that is used by a signature.
      * @param hashToApprove The hash that should be marked as approved for signatures that are verified by this contract.

contracts/GnosisSafeL2.sol → contracts/SafeL2.sol

@@ -1,12 +1,12 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity >=0.7.0 <0.9.0;
 
-import "./GnosisSafe.sol";
+import "./Safe.sol";
 
-/// @title Gnosis Safe - A multisignature wallet with support for confirmations using signed messages based on ERC191.
+/// @title Safe - A multisignature wallet with support for confirmations using signed messages based on ERC191.
 /// @author Stefan George - <stefan@gnosis.io>
 /// @author Richard Meissner - <richard@gnosis.io>
-contract GnosisSafeL2 is GnosisSafe {
+contract SafeL2 is Safe {
     event SafeMultiSigTransaction(
         address to,
         uint256 value,

contracts/base/Executor.sol

@@ -5,6 +5,12 @@ import "../common/Enum.sol";
 /// @title Executor - A contract that can execute transactions
 /// @author Richard Meissner - <richard@gnosis.pm>
 contract Executor {
+    /// @dev Executes either a delegatecall or a call with provided parameters
+    /// @param to Destination address.
+    /// @param value Ether value.
+    /// @param data Data payload.
+    /// @param operation Operation type.
+    /// @return success boolean flag indicating if the call succeeded
     function execute(
         address to,
         uint256 value,

contracts/base/GuardManager.sol

@@ -31,7 +31,7 @@ abstract contract BaseGuard is Guard {
     }
 }
 
-/// @title Fallback Manager - A contract that manages fallback calls made to this contract
+/// @title Guard Manager - A contract that manages transaction guards which perform pre and post-checks on execution by multisig owners
 /// @author Richard Meissner - <richard@gnosis.pm>
 contract GuardManager is SelfAuthorized {
     event ChangedGuard(address guard);

contracts/base/ModuleManager.sol

@@ -20,9 +20,11 @@ contract ModuleManager is SelfAuthorized, Executor {
     function setupModules(address to, bytes memory data) internal {
         require(modules[SENTINEL_MODULES] == address(0), "GS100");
         modules[SENTINEL_MODULES] = SENTINEL_MODULES;
-        if (to != address(0))
+        if (to != address(0)) {
+            require(isContract(to), "GS002");
             // Setup has to complete successfully or transaction fails.
             require(execute(to, 0, data, Enum.Operation.DelegateCall, gasleft()), "GS000");
+        }
     }
 
     /// @dev Allows to add a module to the whitelist.
@@ -67,7 +69,7 @@ contract ModuleManager is SelfAuthorized, Executor {
         // Only whitelisted modules are allowed.
         require(msg.sender != SENTINEL_MODULES && modules[msg.sender] != address(0), "GS104");
         // Execute transaction without further confirmations.
-        success = execute(to, value, data, operation, gasleft());
+        success = execute(to, value, data, operation, type(uint256).max);
         if (success) emit ExecutionFromModuleSuccess(msg.sender);
         else emit ExecutionFromModuleFailure(msg.sender);
     }
@@ -107,27 +109,52 @@ contract ModuleManager is SelfAuthorized, Executor {
     }
 
     /// @dev Returns array of modules.
-    /// @param start Start of the page.
-    /// @param pageSize Maximum number of modules that should be returned.
+    ///      If all entries fit into a single page, the next pointer will be 0x1.
+    ///      If another page is present, next will be the last element of the returned array.
+    /// @param start Start of the page. Has to be a module or start pointer (0x1 address)
+    /// @param pageSize Maximum number of modules that should be returned. Has to be > 0
     /// @return array Array of modules.
     /// @return next Start of the next page.
     function getModulesPaginated(address start, uint256 pageSize) external view returns (address[] memory array, address next) {
+        require(start == SENTINEL_MODULES || isModuleEnabled(start), "GS105");
+        require(pageSize > 0, "GS106");
         // Init array with max page size
         array = new address[](pageSize);
 
         // Populate return array
         uint256 moduleCount = 0;
-        address currentModule = modules[start];
-        while (currentModule != address(0x0) && currentModule != SENTINEL_MODULES && moduleCount < pageSize) {
-            array[moduleCount] = currentModule;
-            currentModule = modules[currentModule];
+        next = modules[start];
+        while (next != address(0) && next != SENTINEL_MODULES && moduleCount < pageSize) {
+            array[moduleCount] = next;
+            next = modules[next];
             moduleCount++;
         }
-        next = currentModule;
+
+        // Because of the argument validation we can assume that
+        // the `currentModule` will always be either a module address
+        // or sentinel address (aka the end). If we haven't reached the end
+        // inside the loop, we need to set the next pointer to the last element
+        // because it skipped over to the next module which is neither included
+        // in the current page nor won't be included in the next one
+        // if you pass it as a start.
+        if (next != SENTINEL_MODULES) {
+            next = array[moduleCount - 1];
+        }
         // Set correct size of returned array
         // solhint-disable-next-line no-inline-assembly
         assembly {
             mstore(array, moduleCount)
         }
     }
+
+    /// @dev Returns true if `account` is a contract.
+    /// @param account The address being queried
+    function isContract(address account) internal view returns (bool) {
+        uint256 size;
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            size := extcodesize(account)
+        }
+        return size > 0;
+    }
 }

contracts/common/Singleton.sol

@@ -2,7 +2,7 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /// @title Singleton - Base for singleton contracts (should always be first super contract)
-///         This contract is tightly coupled to our proxy contract (see `proxies/GnosisSafeProxy.sol`)
+///         This contract is tightly coupled to our proxy contract (see `proxies/SafeProxy.sol`)
 /// @author Richard Meissner - <richard@gnosis.io>
 contract Singleton {
     // singleton always needs to be first declared variable, to ensure that it is at the same location as in the Proxy contract.