Merge branch 'main' into fix-754-signature-length-check

.env.sample

@@ -15,3 +15,5 @@ SAFE_CONTRACT_UNDER_TEST="Safe"
 SOLIDITY_VERSION= # Example: '0.8.19'
 # For running coverage tests, `details` section of solidity settings are required, else could be removed.
 SOLIDITY_SETTINGS= # Example: '{"viaIR":true,"optimizer":{"enabled":true, "details": {"yul": true, "yulDetails": { "optimizerSteps": ""}}}}'
+# Sets hardhat chain id. In general, you don't need this, it's only used for testing the SafeToL2Setup contract.
+HARDHAT_CHAIN_ID=31337

.github/workflows/certora.yml

@@ -15,39 +15,32 @@ jobs:
         runs-on: ubuntu-latest
         strategy:
             matrix:
+
                 rule: ["verifyOwners.sh", "verifySafe.sh", "verifyModules.sh", "verifyNativeTokenRefund.sh"]
 
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
 
             - name: Install python
               uses: actions/setup-python@v4
               with: { python-version: 3.11 }
 
-            - name: Install java
-              uses: actions/setup-java@v3
-              with: { java-version: "17", java-package: jre, distribution: semeru }
-
             - name: Install certora cli
-              run: pip install -Iv certora-cli==7.6.3
+              run: pip install -r certora/requirements.txt
 
             - name: Install solc
               run: |
                   wget https://github.com/ethereum/solidity/releases/download/v0.7.6/solc-static-linux
                   chmod +x solc-static-linux
                   sudo mv solc-static-linux /usr/local/bin/solc7.6
 
-                  wget https://github.com/ethereum/solidity/releases/download/v0.8.19/solc-static-linux
-                  chmod +x solc-static-linux
-                  sudo mv solc-static-linux /usr/local/bin/solc8.19
-
             - name: Verify rule ${{ matrix.rule }}
               run: |
                   cd certora
                   touch applyHarness.patch
                   make munged
                   cd ..
                   echo "key length" ${#CERTORAKEY}
-                  ./certora/scripts/${{ matrix.rule }}
+                  certoraRun certora/conf/${{ matrix.rule }}.conf --wait_for_results=all
               env:
                   CERTORAKEY: ${{ secrets.CERTORA_KEY }}

.github/workflows/ci.yml

@@ -1,14 +1,14 @@
 name: safe-smart-account
 on: [push, pull_request]
 env:
-    NODE_VERSION: 18.17.1
+    NODE_VERSION: 20.16.0
 
 jobs:
     lint-solidity:
         runs-on: ubuntu-latest
         steps:
-            - uses: actions/checkout@v2
-            - uses: actions/setup-node@v2
+            - uses: actions/checkout@v4
+            - uses: actions/setup-node@v4
               with:
                   node-version: ${{ env.NODE_VERSION }}
                   cache: "npm"
@@ -19,8 +19,8 @@ jobs:
     lint-typescript:
         runs-on: ubuntu-latest
         steps:
-            - uses: actions/checkout@v2
-            - uses: actions/setup-node@v2
+            - uses: actions/checkout@v4
+            - uses: actions/setup-node@v4
               with:
                   node-version: ${{ env.NODE_VERSION }}
                   cache: "npm"
@@ -37,8 +37,8 @@ jobs:
         env:
             SAFE_CONTRACT_UNDER_TEST: ${{ matrix.contract-name }}
         steps:
-            - uses: actions/checkout@v2
-            - uses: actions/setup-node@v2
+            - uses: actions/checkout@v4
+            - uses: actions/setup-node@v4
               with:
                   node-version: ${{ env.NODE_VERSION }}
                   cache: "npm"
@@ -71,13 +71,13 @@ jobs:
                 solidity: ["0.7.6", "0.8.24"]
                 include:
                     - solidity: "0.8.24"
-                      settings: '{"viaIR":true,"optimizer":{"enabled":true,"runs":1000000}}'
+                      settings: '{"viaIR":false,"optimizer":{"enabled":true,"runs":1000000}}'
         env:
             SOLIDITY_VERSION: ${{ matrix.solidity }}
             SOLIDITY_SETTINGS: ${{ matrix.settings }}
         steps:
-            - uses: actions/checkout@v2
-            - uses: actions/setup-node@v2
+            - uses: actions/checkout@v4
+            - uses: actions/setup-node@v4
               with:
                   node-version: ${{ env.NODE_VERSION }}
                   cache: "npm"

.gitignore

@@ -18,4 +18,7 @@ typechain-types
 # Certora Formal Verification related files
 .certora_internal
 .certora_recent_jobs.json
-.zip-output-url.txt
+.zip-output-url.txt
+
+# Safe YUL code
+contracts/SafeBytecode.sol

Makefile

@@ -0,0 +1,19 @@
+JQ ?= jq
+
+SAFEYULROOT ?=
+SAFEYULBASE := build/yul/Safe.json
+SAFEYUL     := $(SAFEYULROOT)/$(SAFEYULBASE)
+
+contracts/SafeBytecode.sol: $(SAFEYUL)
+	@echo '// SPDX-License-Identifier: LGPL-3.0-only' >$@
+	@echo 'pragma solidity >=0.7.0 <0.9.0;' >>$@
+	@echo '' >>$@
+	@echo 'contract SafeBytecode {' >>$@
+	@echo '    bytes public constant DEPLOYED_BYTECODE =' >>$@
+	@echo '        hex$(shell $(JQ) '.evm.deployedBytecode.object' $<);' >>$@
+	@echo '}' >>$@
+
+.PHONY: $(SAFEYUL)
+$(SAFEYUL):
+	@test -n "$(SAFEYULROOT)" || ( echo 'SAFEYULROOT not specified'; exit 1 )
+	@$(MAKE) -C $(SAFEYULROOT) $(SAFEYULBASE)

README.md

@@ -48,7 +48,7 @@ To add support for a new network follow the steps of the ``Deploy`` section and
 
 > :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-smart-account/releases).
 
-> **Current version:** The latest release is [v1.3.0-libs.0](https://github.com/safe-global/safe-smart-account/tree/v1.3.0-libs.0) on the commit [767ef36](https://github.com/safe-global/safe-smart-account/commit/767ef36bba88bdbc0c9fe3708a4290cabef4c376)
+> **Current version:** The latest release is [v1.4.1-build.0](https://github.com/safe-global/safe-smart-account/tree/v1.4.1-build.0) on the commit [192c7dc](https://github.com/safe-global/safe-smart-account/commit/192c7dc67290940fcbc75165522bb86a37187069)
 
 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.
 
@@ -84,7 +84,7 @@ Note: Address will vary if contract code is changed or a different Solidity vers
 
 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). 
 
-Safe Smart Account 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 `npm i --save-dev @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.  
+Safe Smart Account 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, run `npm i --save-dev @safe-global/safe-singleton-factory` before deployment. For more information, including deploying the factory to a new network, please refer to the factory repo.  
 
 Note: This will result in different addresses compared to hardhat's default deterministic deployment process.
 

benchmark/utils/setup.ts

@@ -1,7 +1,7 @@
 import { expect } from "chai";
 import hre, { deployments, ethers } from "hardhat";
 import { BigNumberish } from "ethers";
-import { getTokenCallbackHandler, getSafeWithOwners } from "../../test/utils/setup";
+import { getTokenCallbackHandler, getSafe } from "../../test/utils/setup";
 import {
     logGas,
     executeTx,
@@ -25,12 +25,13 @@ const generateTarget = async (owners: number, threshold: number, guardAddress: s
     const fallbackHandler = await getTokenCallbackHandler();
     const fallbackHandlerAddress = await fallbackHandler.getAddress();
     const signers = (await ethers.getSigners()).slice(0, owners);
-    const safe = await getSafeWithOwners(
-        signers.map((owner) => owner.address),
+    const safe = await getSafe({
+        owners: signers.map((owner) => owner.address),
         threshold,
-        fallbackHandlerAddress,
+        fallbackHandler: fallbackHandlerAddress,
         logGasUsage,
         saltNumber,
+
     );
     await executeContractCallWithSigners(safe, safe, "setGuard", [guardAddress], signers.slice(0, threshold));
     return safe;

certora/requirements.txt

@@ -0,0 +1 @@
+certora-cli==7.10.1

certora/specs/Safe2.spec

@@ -0,0 +1,65 @@
+methods {
+    // 
+    function getThreshold() external returns (uint256) envfree;
+    function disableModule(address,address) external;
+    function nonce() external returns (uint256) envfree;
+
+    // harnessed
+    function getModule(address) external returns (address) envfree;
+    function getOwnersCount() external returns (uint256) envfree;
+    function getOwnersCountFromArray() external returns (uint256) envfree;
+
+    // optional
+    function execTransactionFromModuleReturnData(address,uint256,bytes,SafeHarness.Operation) external returns (bool, bytes memory);
+    function execTransactionFromModule(address,uint256,bytes,SafeHarness.Operation) external returns (bool);
+    function execTransaction(address,uint256,bytes,SafeHarness.Operation,uint256,uint256,uint256,address,address,bytes) external returns (bool);
+
+}
+
+definition noHavoc(method f) returns bool =
+    f.selector != sig:execTransactionFromModuleReturnData(address,uint256,bytes,SafeHarness.Operation).selector
+    && f.selector != sig:execTransactionFromModule(address,uint256,bytes,SafeHarness.Operation).selector 
+    && f.selector != sig:execTransaction(address,uint256,bytes,SafeHarness.Operation,uint256,uint256,uint256,address,address,bytes).selector;
+
+definition reachableOnly(method f) returns bool =
+    f.selector != sig:setup(address[],uint256,address,bytes,address,address,uint256,address).selector
+    && f.selector != sig:simulateAndRevert(address,bytes).selector;
+
+definition ownerUpdatingFunctions(method f) returns bool = 
+    f.selector != sig:addOwnerWithThreshold(address,uint256).selector
+        && f.selector != sig:removeOwner(address,address,uint256).selector;
+
+
+invariant safeIsSetup() getThreshold() > 0;
+
+invariant safeOwnerCountConsistency() getOwnersCount() == getOwnersCountFromArray();
+
+invariant threholdShouldBeLessThanOwners() getOwnersCount() >= getThreshold();
+
+invariant safeOwnerCannotBeItself(env e) !isOwner(e, currentContract);
+
+invariant safeOwnerCannotBeSentinelAddress(env e) !isOwner(e, 1);
+
+rule safeOwnerCountCannotBeUpdatedByNonOwnerUpdatingFunctions(method f) filtered {
+  f -> ownerUpdatingFunctions(f)
+}
+{
+    requireInvariant safeIsSetup;
+    uint256 ownerCountBefore = getOwnersCount();
+    calldataarg args; env e;
+    f(e, args);
+    uint256 ownerCountAfter = getOwnersCount();
+    assert ownerCountAfter == ownerCountBefore;
+}
+
+rule onlyEnableModuleFunctionCanAddModule(method f, address moduleAddress) filtered {
+  f -> f.selector != sig: enableModule(address).selector
+}
+{
+    requireInvariant safeIsSetup;
+    calldataarg args; env e;
+    require !isModuleEnabled(e, moduleAddress);
+    f(e, args);
+    assert !isModuleEnabled(e, moduleAddress);
+
+}

contracts/Safe.sol

@@ -15,6 +15,8 @@ import {ISignatureValidator, ISignatureValidatorConstants} from "./interfaces/IS
 import {SafeMath} from "./external/SafeMath.sol";
 import {ISafe} from "./interfaces/ISafe.sol";
 
+import "./SafeBytecode.sol";
+
 /**
  * @title Safe - A multisignature wallet with support for confirmations using signed messages based on EIP-712.
  * @dev Most important concepts:
@@ -69,13 +71,24 @@ contract Safe is
     mapping(address => mapping(bytes32 => uint256)) public override approvedHashes;
 
     // This constructor ensures that this contract can only be used as a singleton for Proxy contracts
-    constructor() {
+    constructor(address byteCode) {
         /**
          * By setting the threshold it is not possible to call setup anymore,
          * so we create a Safe with 0 owners and threshold 1.
          * This is an unusable Safe, perfect for the singleton
          */
         threshold = 1;
+
+        if (byteCode != address(0)) {
+            bytes memory code = SafeBytecode(byteCode).DEPLOYED_BYTECODE();
+
+            /* solhint-disable no-inline-assembly */
+            /// @solidity memory-safe-assembly
+            assembly {
+                return(add(code, 32), mload(code))
+            }
+            /* solhint-enable no-inline-assembly */
+        }
     }
 
     /**

contracts/SafeL2.sol

@@ -37,6 +37,7 @@ contract SafeL2 is Safe {
      * @inheritdoc Safe
      */
     function onBeforeExecTransaction(
+
         address to,
         uint256 value,
         bytes calldata data,
@@ -69,8 +70,16 @@ contract SafeL2 is Safe {
 
     /**
      * @inheritdoc ModuleManager
+
      */
-    function onBeforeExecTransactionFromModule(address to, uint256 value, bytes memory data, Enum.Operation operation) internal override {
+    function onBeforeExecTransactionFromModule(
+        address to,
+        uint256 value,
+        bytes memory data,
+        Enum.Operation operation,
+        bytes memory /* context */
+    ) internal override {
         emit SafeModuleTransaction(msg.sender, to, value, data, operation);
+
     }
 }