Compose TimelockGuard and LivenessModule2 into a single contract

Also makes the parent contracts abstract in order to prevent deploying
them individually.

The benefit of this approach is that it ensures the logic is available
in both contracts, while ensuring that their state and logic are kept
separate thus reducing the complexity and review effort.

Author: maurelian

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

@@ -11,6 +11,7 @@ import { Enum as SafeOps } from "safe-contracts/common/Enum.sol";
 
 import { DeployUtils } from "scripts/libraries/DeployUtils.sol";
 
+import { SafeExtensions } from "src/safe/SafeExtensions.sol";
 import { LivenessModule2 } from "src/safe/LivenessModule2.sol";
 import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
 
@@ -242,7 +243,7 @@ contract DeployOwnership is Deploy {
     ///         Note this function does not have the broadcast modifier.
     function deployLivenessModule() public returns (address addr_) {
         // Deploy the singleton LivenessModule2 (no parameters needed)
-        addr_ = address(new LivenessModule2());
+        addr_ = address(new SafeExtensions());
 
         artifacts.save("LivenessModule2", address(addr_));
         console.log("New LivenessModule2 deployed at %s", address(addr_));
@@ -324,7 +325,7 @@ contract DeployOwnership is Deploy {
 
         // Verify the module was configured correctly
         (uint256 configuredPeriod, address configuredFallback) =
-            LivenessModule2(livenessModule).safeConfigs(address(safe));
+            SafeExtensions(livenessModule).safeConfigs(address(safe));
         require(
             configuredPeriod == exampleCouncilConfig.livenessModuleConfig.livenessInterval,
             "DeployOwnership: configured liveness interval must match expected value"

packages/contracts-bedrock/src/safe/LivenessModule2.sol

@@ -14,10 +14,11 @@ import { ISemver } from "interfaces/universal/ISemver.sol";
 ///         when the Safe becomes unresponsive. The fallback owner can initiate a challenge,
 ///         and if the Safe doesn't respond within the challenge period, ownership transfers
 ///         to the fallback owner.
-/// @dev This is a singleton contract. To use it:
+/// @dev This is an abstract contract. Concrete implementations must provide version info.
+///      To use it:
 ///      1. The Safe must first enable this module using ModuleManager.enableModule()
 ///      2. The Safe must then configure the module by calling configure() with params
-contract LivenessModule2 is ISemver {
+abstract contract LivenessModule2 is ISemver {
     /// @notice Configuration for a Safe's liveness module
     struct ModuleConfig {
         uint256 livenessResponsePeriod;
@@ -81,9 +82,6 @@ contract LivenessModule2 is ISemver {
     /// @notice Emitted when ownership is transferred to the fallback owner
     event ChallengeSucceeded(address indexed safe, address fallbackOwner);
 
-    /// @notice Semantic version.
-    /// @custom:semver 2.0.0
-    string public constant version = "2.0.0";
 
     /// @notice Returns challenge_start_time + liveness_response_period if challenge exists, or
     /// 0 if not

packages/contracts-bedrock/src/safe/SafeExtensions.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+// Safe Extensions
+import { LivenessModule2 } from "./LivenessModule2.sol";
+import { TimelockGuard } from "./TimelockGuard.sol";
+
+// Interfaces
+import { ISemver } from "interfaces/universal/ISemver.sol";
+
+/// @title SafeExtensions
+/// @notice Combined Safe extensions providing both liveness module and timelock guard functionality
+/// @dev This contract can be enabled simultaneously as both a module and a guard on a Safe:
+///      - As a module: provides liveness challenge functionality to prevent multisig deadlock
+///      - As a guard: provides timelock functionality for transaction delays and cancellation
+contract SafeExtensions is LivenessModule2, TimelockGuard {
+    /// @notice Semantic version.
+    /// @custom:semver 1.0.0
+    string public constant version = "1.0.0";
+}

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

@@ -11,17 +11,18 @@ import { ISemver } from "interfaces/universal/ISemver.sol";
 
 /// @title TimelockGuard
 /// @notice This guard provides timelock functionality for Safe transactions
-/// @dev This is a singleton contract. To use it:
+/// @dev This is an abstract contract. Concrete implementations must provide version info.
+///      To use it:
 ///      1. The Safe must first enable this guard using GuardManager.setGuard()
 ///      2. The Safe must then configure the guard by calling configureTimelockGuard()
-contract TimelockGuard is IGuard, ISemver {
+abstract contract TimelockGuard is IGuard, ISemver {
     /// @notice Configuration for a Safe's timelock guard
     struct GuardConfig {
         uint256 timelockDelay;
     }
 
     /// @notice Mapping from Safe address to its guard configuration
-    mapping(address => GuardConfig) public safeConfigs;
+    mapping(address => GuardConfig) public guardConfigs;
 
     /// @notice Mapping from Safe address to its current cancellation threshold
     mapping(address => uint256) public safeCancellationThreshold;
@@ -44,17 +45,13 @@ contract TimelockGuard is IGuard, ISemver {
     /// @notice Emitted when a Safe clears the guard configuration
     event GuardCleared(address indexed safe);
 
-    /// @notice Semantic version.
-    /// @custom:semver 1.0.0
-    string public constant version = "1.0.0";
 
     /// @notice Returns the timelock delay for a given Safe
     /// @dev MUST never revert
     /// @param _safe The Safe address to query
     /// @return The timelock delay in seconds
     function viewTimelockGuardConfiguration(address _safe) public view returns (uint256) {
-        // Q: What should this return if the guard is not enabled?
-        return safeConfigs[_safe].timelockDelay;
+        return guardConfigs[_safe].timelockDelay;
     }
 
     /// @notice Configure the contract as a timelock guard by setting the timelock delay
@@ -77,7 +74,7 @@ contract TimelockGuard is IGuard, ISemver {
         }
 
         // Store the configuration for this safe
-        safeConfigs[msg.sender].timelockDelay = _timelockDelay;
+        guardConfigs[msg.sender].timelockDelay = _timelockDelay;
 
         // Initialize cancellation threshold to 1
         safeCancellationThreshold[msg.sender] = 1;
@@ -91,7 +88,7 @@ contract TimelockGuard is IGuard, ISemver {
     /// @dev MUST emit a GuardCleared event
     function clearTimelockGuard() external {
         // Check if the calling safe has configuration set
-        if (safeConfigs[msg.sender].timelockDelay == 0) {
+        if (guardConfigs[msg.sender].timelockDelay == 0) {
             revert TimelockGuard_GuardNotConfigured();
         }
 
@@ -101,7 +98,7 @@ contract TimelockGuard is IGuard, ISemver {
         }
 
         // Erase the configuration data for this safe
-        delete safeConfigs[msg.sender];
+        delete guardConfigs[msg.sender];
         delete safeCancellationThreshold[msg.sender];
 
         emit GuardCleared(msg.sender);
@@ -119,7 +116,7 @@ contract TimelockGuard is IGuard, ISemver {
         }
 
         // Return 0 if not configured
-        if (safeConfigs[_safe].timelockDelay == 0) {
+        if (guardConfigs[_safe].timelockDelay == 0) {
             return 0;
         }
 

packages/contracts-bedrock/test/safe/LivenessModule2.t.sol

@@ -7,6 +7,14 @@ import "test/safe-tools/SafeTestTools.sol";
 
 import { LivenessModule2 } from "src/safe/LivenessModule2.sol";
 
+/// @title ConcreteLivenessModule2
+/// @notice Concrete implementation of LivenessModule2 for testing
+contract ConcreteLivenessModule2 is LivenessModule2 {
+    /// @notice Semantic version.
+    /// @custom:semver 2.0.0-test
+    string public constant version = "2.0.0-test";
+}
+
 /// @title LivenessModule2_TestInit
 /// @notice Reusable test initialization for `LivenessModule2` tests.
 contract LivenessModule2_TestInit is Test, SafeTestTools {
@@ -24,7 +32,7 @@ contract LivenessModule2_TestInit is Test, SafeTestTools {
     uint256 constant NUM_OWNERS = 5;
     uint256 constant THRESHOLD = 3;
 
-    LivenessModule2 livenessModule2;
+    ConcreteLivenessModule2 livenessModule2;
     SafeInstance safeInstance;
     address fallbackOwner;
     address[] owners;
@@ -34,7 +42,7 @@ contract LivenessModule2_TestInit is Test, SafeTestTools {
         vm.warp(INIT_TIME);
 
         // Deploy the singleton LivenessModule2
-        livenessModule2 = new LivenessModule2();
+        livenessModule2 = new ConcreteLivenessModule2();
 
         // Create Safe owners
         (address[] memory _owners, uint256[] memory _keys) = SafeTestLib.makeAddrsAndKeys("owners", NUM_OWNERS);

packages/contracts-bedrock/test/safe/TimelockGuard.t.sol

@@ -9,6 +9,14 @@ import "test/safe-tools/SafeTestTools.sol";
 
 import { TimelockGuard } from "src/safe/TimelockGuard.sol";
 
+/// @title ConcreteTimelockGuard
+/// @notice Concrete implementation of TimelockGuard for testing
+contract ConcreteTimelockGuard is TimelockGuard {
+    /// @notice Semantic version.
+    /// @custom:semver 1.0.0-test
+    string public constant version = "1.0.0-test";
+}
+
 /// @title TimelockGuard_TestInit
 /// @notice Reusable test initialization for `TimelockGuard` tests.
 contract TimelockGuard_TestInit is Test, SafeTestTools {
@@ -24,7 +32,7 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {
     uint256 constant THRESHOLD = 3;
     uint256 constant ONE_YEAR = 365 days;
 
-    TimelockGuard timelockGuard;
+    ConcreteTimelockGuard timelockGuard;
     SafeInstance safeInstance;
     SafeInstance safeInstance2;
     address[] owners;
@@ -34,7 +42,7 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {
         vm.warp(INIT_TIME);
 
         // Deploy the singleton TimelockGuard
-        timelockGuard = new TimelockGuard();
+        timelockGuard = new ConcreteTimelockGuard();
 
         // Create Safe owners
         (address[] memory _owners, uint256[] memory _keys) = SafeTestLib.makeAddrsAndKeys("owners", NUM_OWNERS);