Refactor Storage Slot Usage in Contracts (#979)

This pull request introduces new top-level constants `SafeStorage`
library to centralize and standardize the management of storage slot
constants across multiple contracts.

Why:
To prevent copy-paste, examples would be:
- Our own `HandlerContext` contract changed in the PR
- [guardrail
contract](https://github.com/safe-research/guardrail/blob/13d978060fc245951cf4d0ae02c0110c9e7d8c1f/src/Guardrail.sol#L27)

This change reduces gas costs by inlining constants in assembly where
they previously were not.

---------

Co-authored-by: Nicholas Rodrigues Lordello <nick@safe.global>

Author: mmv08

certora/applyHarness.patch

@@ -1,6 +1,6 @@
 diff -druN Safe.sol Safe.sol
---- Safe.sol	2025-05-27 10:10:59
-+++ Safe.sol	2025-05-27 10:10:59
+--- Safe.sol	2025-07-09 12:21:55.831312348 +0000
++++ Safe.sol	2025-07-09 12:45:47.086078483 +0000
 @@ -75,22 +75,25 @@
           * so we create a Safe with 0 owners and threshold 1.
           * This is an unusable Safe, perfect for the singleton
@@ -31,7 +31,7 @@ diff -druN Safe.sol Safe.sol
          // Emit the setup event optimistically. This ensures that changes such as `addOwner` and `changeThreshold` that are part
          // of the  `to.delegatecall(data)` that happen in the `setupModules` call emit events in order relative to the setup
          // event, making it easier for off-chain indexers to reliably reconstruct the Safe configuration.
-@@ -397,9 +400,6 @@
+@@ -398,9 +401,6 @@
          return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, chainId, this));
      }
 
@@ -41,7 +41,7 @@ diff -druN Safe.sol Safe.sol
      function getTransactionHash(
          address to,
          uint256 value,
-@@ -411,7 +411,9 @@
+@@ -412,7 +412,9 @@
          address gasToken,
          address refundReceiver,
          uint256 _nonce
@@ -52,7 +52,7 @@ diff -druN Safe.sol Safe.sol
          bytes32 domainHash = domainSeparator();
 
          // We opted for using assembly code here, because the way Solidity compiler we use (0.7.6) allocates memory is
-@@ -466,7 +468,8 @@
+@@ -467,7 +469,8 @@
              // Store the domain separator
              mstore(add(ptr, 32), domainHash)
              // Calculate the hash
@@ -63,8 +63,8 @@ diff -druN Safe.sol Safe.sol
          /* solhint-enable no-inline-assembly */
      }
 diff -druN base/Executor.sol base/Executor.sol
---- base/Executor.sol	2025-05-27 10:10:59
-+++ base/Executor.sol	2025-05-27 10:10:59
+--- base/Executor.sol	2025-07-03 08:51:19.308739177 +0000
++++ base/Executor.sol	2025-07-09 12:45:47.086392895 +0000
 @@ -26,12 +26,8 @@
          uint256 txGas
      ) internal returns (bool success) {
@@ -81,9 +81,9 @@ diff -druN base/Executor.sol base/Executor.sol
              /* solhint-disable no-inline-assembly */
              /// @solidity memory-safe-assembly
 diff -druN base/FallbackManager.sol base/FallbackManager.sol
---- base/FallbackManager.sol	2025-05-27 10:10:59
-+++ base/FallbackManager.sol	2025-05-27 10:10:59
-@@ -61,7 +61,8 @@
+--- base/FallbackManager.sol	2025-07-09 12:42:34.885183896 +0000
++++ base/FallbackManager.sol	2025-07-09 12:45:47.086663906 +0000
+@@ -60,7 +60,8 @@
              // not going beyond the scratch space, etc)
              // Solidity docs: https://docs.soliditylang.org/en/latest/assembly.html#memory-safety
 
@@ -94,8 +94,8 @@ diff -druN base/FallbackManager.sol base/FallbackManager.sol
              if iszero(handler) {
                  return(0, 0)
 diff -druN interfaces/ISafe.sol interfaces/ISafe.sol
---- interfaces/ISafe.sol	2025-05-27 10:10:59
-+++ interfaces/ISafe.sol	2025-05-27 10:10:59
+--- interfaces/ISafe.sol	2025-07-03 08:51:19.312340984 +0000
++++ interfaces/ISafe.sol	2025-07-09 12:45:47.087200687 +0000
 @@ -117,7 +117,7 @@
       */
      function domainSeparator() external view returns (bytes32);

certora/harnesses/ExtensibleFallbackHandlerHarness.sol

@@ -3,9 +3,7 @@ import "../munged/handler/ExtensibleFallbackHandler.sol";
 import {ISafe} from "../munged/interfaces/ISafe.sol";
 
 contract ExtensibleFallbackHandlerHarness is ExtensibleFallbackHandler {
-
     function getSafeMethod(ISafe safe, bytes4 selector) public view returns (bytes32) {
         return safeMethods[safe][selector];
     }
-
-}
+}

certora/harnesses/SafeHarness.sol

@@ -3,7 +3,7 @@ import "../munged/Safe.sol";
 import {SafeMath} from "../munged/external/SafeMath.sol";
 import {ISafe, IStaticFallbackMethod, IFallbackMethod, ExtensibleBase} from "../munged/handler/extensible/ExtensibleBase.sol";
 import {IFallbackHandler, FallbackHandler} from "../munged/handler/extensible/FallbackHandler.sol";
-
+import {FALLBACK_HANDLER_STORAGE_SLOT} from "../munged/libraries/SafeStorage.sol";
 
 contract SafeHarness is Safe {
     constructor(
@@ -35,8 +35,8 @@ contract SafeHarness is Safe {
         }
     }
 
-    function numSigsSufficient(bytes memory signatures,uint256 requiredSignatures) public pure returns (bool) {
-        return (signatures.length >= SafeMath.mul(requiredSignatures,65));
+    function numSigsSufficient(bytes memory signatures, uint256 requiredSignatures) public pure returns (bool) {
+        return (signatures.length >= SafeMath.mul(requiredSignatures, 65));
     }
 
     // harnessed getters
@@ -74,14 +74,14 @@ contract SafeHarness is Safe {
 
     function getFallbackHandler() public view returns (address) {
         address handler;
-        assembly{
+        assembly {
             handler := sload(FALLBACK_HANDLER_STORAGE_SLOT)
         }
-        return handler ;
+        return handler;
     }
 
     function callSetSafeMethod(bytes4 selector, bytes32 newMethod) public {
-        IFallbackHandler(address(this)).setSafeMethod(selector,newMethod);
+        IFallbackHandler(address(this)).setSafeMethod(selector, newMethod);
     }
 
     function callDummyHandler(bytes4 selector) public {

contracts/base/FallbackManager.sol

@@ -3,15 +3,14 @@ pragma solidity >=0.7.0 <0.9.0;
 
 import {SelfAuthorized} from "../common/SelfAuthorized.sol";
 import {IFallbackManager} from "../interfaces/IFallbackManager.sol";
+// solhint-disable-next-line no-unused-import
+import {FALLBACK_HANDLER_STORAGE_SLOT} from "../libraries/SafeStorage.sol";
 
 /**
  * @title Fallback Manager - A contract managing fallback calls made to this contract
  * @author Richard Meissner - @rmeissner
  */
 abstract contract FallbackManager is SelfAuthorized, IFallbackManager {
-    // keccak256("fallback_manager.handler.address")
-    bytes32 internal constant FALLBACK_HANDLER_STORAGE_SLOT = 0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5;
-
     /**
      *  @notice Internal function to set the fallback handler.
      *  @param handler contract to handle fallback calls.

contracts/base/GuardManager.sol

@@ -6,6 +6,8 @@ import {SelfAuthorized} from "./../common/SelfAuthorized.sol";
 import {IERC165} from "./../interfaces/IERC165.sol";
 import {IGuardManager} from "./../interfaces/IGuardManager.sol";
 import {Enum} from "./../libraries/Enum.sol";
+// solhint-disable-next-line no-unused-import
+import {GUARD_STORAGE_SLOT} from "../libraries/SafeStorage.sol";
 
 /**
  * @title ITransactionGuard Interface
@@ -62,15 +64,13 @@ abstract contract BaseTransactionGuard is ITransactionGuard {
  * @author Richard Meissner - @rmeissner
  */
 abstract contract GuardManager is SelfAuthorized, IGuardManager {
-    // keccak256("guard_manager.guard.address")
-    bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
-
     /**
      * @inheritdoc IGuardManager
      */
     function setGuard(address guard) external override authorized {
         if (guard != address(0) && !ITransactionGuard(guard).supportsInterface(type(ITransactionGuard).interfaceId))
             revertWithError("GS300");
+
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {

contracts/base/ModuleManager.sol

@@ -5,6 +5,8 @@ import {SelfAuthorized} from "./../common/SelfAuthorized.sol";
 import {IERC165} from "./../interfaces/IERC165.sol";
 import {IModuleManager} from "./../interfaces/IModuleManager.sol";
 import {Enum} from "./../libraries/Enum.sol";
+// solhint-disable-next-line no-unused-import
+import {MODULE_GUARD_STORAGE_SLOT} from "./../libraries/SafeStorage.sol";
 import {Executor} from "./Executor.sol";
 
 /**
@@ -61,9 +63,6 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
     //      2. `modules[last_module]` points back to SENTINEL_MODULES
     address internal constant SENTINEL_MODULES = address(0x1);
 
-    // keccak256("module_manager.module_guard.address")
-    bytes32 internal constant MODULE_GUARD_STORAGE_SLOT = 0xb104e0b93118902c651344349b610029d694cfdec91c589c91ebafbcd0289947;
-
     mapping(address => address) internal modules;
 
     /**
@@ -217,10 +216,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
 
         /**
           Because of the argument validation, we can assume that the loop will always iterate over the valid module list values
-          and the `next` variable will either be an enabled module or a sentinel address (signalling the end). 
-          
+          and the `next` variable will either be an enabled module or a sentinel address (signalling the end).
+
           If we haven't reached the end inside the loop, we need to set the next pointer to the last element of the modules array
-          because the `next` variable (which is a module by itself) acting as a pointer to the start of the next page is neither 
+          because the `next` variable (which is a module by itself) acting as a pointer to the start of the next page is neither
           included to the current page, nor will it be included in the next one if you pass it as a start.
         */
         if (next != SENTINEL_MODULES) {
@@ -259,11 +258,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
         if (moduleGuard != address(0) && !IModuleGuard(moduleGuard).supportsInterface(type(IModuleGuard).interfaceId))
             revertWithError("GS301");
 
-        bytes32 slot = MODULE_GUARD_STORAGE_SLOT;
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {
-            sstore(slot, moduleGuard)
+            sstore(MODULE_GUARD_STORAGE_SLOT, moduleGuard)
         }
         /* solhint-enable no-inline-assembly */
         emit ChangedModuleGuard(moduleGuard);
@@ -274,11 +272,10 @@ abstract contract ModuleManager is SelfAuthorized, Executor, IModuleManager {
      * @return moduleGuard The address of the guard
      */
     function getModuleGuard() internal view returns (address moduleGuard) {
-        bytes32 slot = MODULE_GUARD_STORAGE_SLOT;
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {
-            moduleGuard := sload(slot)
+            moduleGuard := sload(MODULE_GUARD_STORAGE_SLOT)
         }
         /* solhint-enable no-inline-assembly */
     }

contracts/handler/HandlerContext.sol

@@ -2,6 +2,7 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 import {ISafe} from "../interfaces/ISafe.sol";
+import {FALLBACK_HANDLER_STORAGE_SLOT} from "../libraries/SafeStorage.sol";
 
 /**
  * @title Handler Context - Allows the fallback handler to extract additional context from the calldata
@@ -11,12 +12,6 @@ import {ISafe} from "../interfaces/ISafe.sol";
  * @author Richard Meissner - @rmeissner
  */
 abstract contract HandlerContext {
-    /**
-     * @dev The storage slot used for storing the currently configured fallback handler address.
-     *      Precomputed value of: `keccak256("fallback_manager.handler.address")`.
-     */
-    bytes32 internal constant FALLBACK_HANDLER_STORAGE_SLOT = 0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5;
-
     /**
      * @notice A modifier that reverts if not called by a Safe as a fallback handler.
      * @dev Note that this modifier does a **best effort** attempt at not allowing calls that are

contracts/libraries/SafeStorage.sol

@@ -22,3 +22,21 @@ abstract contract SafeStorage {
     mapping(bytes32 => uint256) internal signedMessages;
     mapping(address => mapping(bytes32 => uint256)) internal approvedHashes;
 }
+
+/**
+ * @dev The storage slot used for storing the currently configured fallback handler address.
+ *      Precomputed value of: `keccak256("fallback_manager.handler.address")`.
+ */
+bytes32 constant FALLBACK_HANDLER_STORAGE_SLOT = 0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5;
+
+/**
+ * @dev The storage slot used for storing the currently configured transaction guard.
+ *      Precomputed value of: `keccak256("guard_manager.guard.address")`.
+ */
+bytes32 constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
+
+/**
+ * @dev The storage slot used for storing the currently configured module guard.
+ *      Precomputed value of: `keccak256("module_manager.module_guard.address")`.
+ */
+bytes32 constant MODULE_GUARD_STORAGE_SLOT = 0xb104e0b93118902c651344349b610029d694cfdec91c589c91ebafbcd0289947;