Require that msgSender be an owner of the Safe (#17900)

* Add SaferSafes as child of the module and guard

* Add ISaferSafes

* Test comment and assertion fixes

* Improve comments

* Make LivenessModule2 and TimelockGuard abstract

Move semver to SaferSafes

semver lock

* fix test contract name

* Move semver to SaferSafes

* Disable the guard and module upon ownership transfer

* Add _disableThisGuard function

* Update tests

* Add config resets

* fmt

* fix test_changeOwnershipToFallback_canRechallenge_succeeds

* Simplify by clearing config directly

* Put _disableThisGuard into child contract

* Add timelockDelay reset on _disableThisGuard

* semver-lock

* Move _disableThisGuard logic into TimelockGuard

* clear livenessSafeConfig at tend of _disableThisModule

* Clarify use of SENTINEL_OWNER

* Fix the ordering of the disableGuard and disableModule calls

* semver-lock

* remove unused imports

* rename _disableThisGuard to _disableGuard

* bump semver

* Add test to remove unrelated guard

* Add SENTINEL_MODULE constant

* Clean up using ternary if

* Reset cancellationThreshold to 0 on changeOwnership

* Fix moduleFound if/else handling

* Clear pending transactions

* Pre-pr fixes

* Add test contract to test name lint exclusions

* fix name of test contract

* Move _disableGuard impl into TimelockGuard

* Add missing natspec

* Add gas limit testing on changeOwnershipToFallback

* Remove interfaces for abstract contracts

* Move state changes out into internal _clearLivenessModule

* Improve names on the internal _disableX methods

* Add clearTimelockGuard function

* Add _disableGuard helper to TLG tests

* Limit number of transactions cancelled to 100

* Revert "Remove interfaces for abstract contracts"

This reverts commit bd03288.

* Move livenessModule2 address into TestUtils

Reduces diff a bit

* Reduce diff somewhat

* Remove unused arg

* Update packages/contracts-bedrock/src/safe/TimelockGuard.sol

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* Fix iface

* update abi for iface fix

* Do not clear or disable the module during ownership transfer

* Fix inaccurate comment on _disableAndClearGuard

* Further improve comment

* remove unused import

* fix test name

* Do not clear guard during changeOwnershipToFallback

* Remove unused SENTINEL_MODULE var

* Remove dangling comment

* Revert "Remove dangling comment"

This reverts commit d266d12.

* Fix whitespace

* remove unnecessary internal _clearTimelockGuard function

It's no longer reused in the change ownership call.

* Address feedback

* Add missing assertion

* Move guard slot into constants

* semver-lock

* Remove LivenessModule from semver-lock

* fix: fmt, semver-lock, unused imports

* Remove unused variable

* fix semver lock by resetting old LivenessModule

* fix unused import

* Require that msgSender be an owner of the Safe

* fix compiler error

* Fix placement of _msgSender check

* semver-lock

* Add TimelockGuard_NotOwner test

* Bump semver

* Add test comment, make into fuzz test

* Improvements to SaferSafes styling (#17903)

* Add public getter livenessSafeConfiguration to return a struct rather than tuple

* Use Safe as input type to mappings and functions on LivenessModule2

* Add dividers based on function type

* fmt

* snapshots

* Remove conditional return of 0 in the cancellationThreshold if the guard is not enabled

* rename timelockConfiguration func to timelockDelay

* semver-lock

* Add missing natspec on tests and convert to fuzzing where possible

* fix import and abi snapshot

* fix: off by one error in challenge period test

* fix test name

---------

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

Author: maurelian

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol

@@ -324,14 +324,14 @@ contract DeployOwnership is Deploy {
         removeDeployerFromSafe({ _name: "SecurityCouncilSafe", _newThreshold: exampleCouncilConfig.safeConfig.threshold });
 
         // Verify the module was configured correctly
-        (uint256 configuredPeriod, address configuredFallback) =
-            LivenessModule2(livenessModule).livenessSafeConfiguration(address(safe));
+        LivenessModule2.ModuleConfig memory verifyConfig =
+            LivenessModule2(livenessModule).livenessSafeConfiguration(safe);
         require(
-            configuredPeriod == exampleCouncilConfig.livenessModuleConfig.livenessInterval,
+            verifyConfig.livenessResponsePeriod == exampleCouncilConfig.livenessModuleConfig.livenessInterval,
             "DeployOwnership: configured liveness interval must match expected value"
         );
         require(
-            configuredFallback == exampleCouncilConfig.livenessModuleConfig.fallbackOwner,
+            verifyConfig.fallbackOwner == exampleCouncilConfig.livenessModuleConfig.fallbackOwner,
             "DeployOwnership: configured fallback owner must match expected value"
         );
 

packages/contracts-bedrock/snapshots/abi/SaferSafes.json

@@ -49,7 +49,7 @@
   {
     "inputs": [
       {
-        "internalType": "address",
+        "internalType": "contract GnosisSafe",
         "name": "_safe",
         "type": "address"
       }
@@ -62,7 +62,7 @@
   {
     "inputs": [
       {
-        "internalType": "address",
+        "internalType": "contract GnosisSafe",
         "name": "",
         "type": "address"
       }
@@ -81,7 +81,7 @@
   {
     "inputs": [
       {
-        "internalType": "address",
+        "internalType": "contract GnosisSafe",
         "name": "_safe",
         "type": "address"
       }
@@ -163,7 +163,7 @@
       },
       {
         "internalType": "address",
-        "name": "",
+        "name": "_msgSender",
         "type": "address"
       }
     ],
@@ -227,7 +227,7 @@
   {
     "inputs": [
       {
-        "internalType": "address",
+        "internalType": "contract GnosisSafe",
         "name": "_safe",
         "type": "address"
       }
@@ -246,22 +246,29 @@
   {
     "inputs": [
       {
-        "internalType": "address",
-        "name": "",
+        "internalType": "contract GnosisSafe",
+        "name": "_safe",
         "type": "address"
       }
     ],
     "name": "livenessSafeConfiguration",
     "outputs": [
       {
-        "internalType": "uint256",
-        "name": "livenessResponsePeriod",
-        "type": "uint256"
-      },
-      {
-        "internalType": "address",
-        "name": "fallbackOwner",
-        "type": "address"
+        "components": [
+          {
+            "internalType": "uint256",
+            "name": "livenessResponsePeriod",
+            "type": "uint256"
+          },
+          {
+            "internalType": "address",
+            "name": "fallbackOwner",
+            "type": "address"
+          }
+        ],
+        "internalType": "struct LivenessModule2.ModuleConfig",
+        "name": "",
+        "type": "tuple"
       }
     ],
     "stateMutability": "view",
@@ -560,7 +567,7 @@
         "type": "address"
       }
     ],
-    "name": "timelockConfiguration",
+    "name": "timelockDelay",
     "outputs": [
       {
         "internalType": "uint256",
@@ -902,6 +909,11 @@
     "name": "TimelockGuard_InvalidVersion",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "TimelockGuard_NotOwner",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "TimelockGuard_TransactionAlreadyCancelled",

packages/contracts-bedrock/snapshots/semver-lock.json

@@ -208,8 +208,8 @@
     "sourceCodeHash": "0x950725f8b9ad9bb3b6b5e836f67e18db824a7864bac547ee0eeba88ada3de0e9"
   },
   "src/safe/SaferSafes.sol:SaferSafes": {
-    "initCodeHash": "0x68567829042bbd450ffd477848ddcda288e36714846fdcc02c315f2d0d332da6",
-    "sourceCodeHash": "0x06fc6d5df3c5769b645ff319d8b94415e501711c19cb0b1bce2631771d22294f"
+    "initCodeHash": "0xaa17bb150c9bcf19675a33e9762b050148aceae9f6a9a6ba020fc6947ebaab39",
+    "sourceCodeHash": "0xc4201612048ff051ed795521efa3eece1a6556f2c514a268b180d84a2ad8b2d1"
   },
   "src/universal/OptimismMintableERC20.sol:OptimismMintableERC20": {
     "initCodeHash": "0x3c85eed0d017dca8eda6396aa842ddc12492587b061e8c756a8d32c4610a9658",

packages/contracts-bedrock/snapshots/storageLayout/SaferSafes.json

@@ -1,17 +1,17 @@
 [
   {
     "bytes": "32",
-    "label": "livenessSafeConfiguration",
+    "label": "_livenessSafeConfiguration",
     "offset": 0,
     "slot": "0",
-    "type": "mapping(address => struct LivenessModule2.ModuleConfig)"
+    "type": "mapping(contract GnosisSafe => struct LivenessModule2.ModuleConfig)"
   },
   {
     "bytes": "32",
     "label": "challengeStartTime",
     "offset": 0,
     "slot": "1",
-    "type": "mapping(address => uint256)"
+    "type": "mapping(contract GnosisSafe => uint256)"
   },
   {
     "bytes": "32",