ctb: Move threshold check from LivenessModule constructor (#10516)

* ctb: Update snapshots and semver-lock

* ctb: Move threshold check from LivenessModule constructor

Perform the check in the deployment scripts instead.

* ctb: Add check to LM deploy script

* ctb: Update snapshots and semver-lock

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

@@ -186,6 +186,14 @@ contract DeployOwnership is Deploy {
         _callViaSafe({ _safe: safe, _target: address(safe), _data: abi.encodeCall(GuardManager.setGuard, (guard)) });
         console.log("LivenessGuard setup on SecurityCouncilSafe");
 
+        // Deploy and add the Liveness Module.
+        address livenessModule = deployLivenessModule();
+        _callViaSafe({
+            _safe: safe,
+            _target: address(safe),
+            _data: abi.encodeCall(ModuleManager.enableModule, (livenessModule))
+        });
+
         // Remove the deployer address (msg.sender) which was used to setup the Security Council Safe thus far
         // this call is also used to update the threshold.
         // Because deploySafe() always adds msg.sender first (if keepDeployer is true), we know that the previousOwner
@@ -197,17 +205,11 @@ contract DeployOwnership is Deploy {
                 OwnerManager.removeOwner, (SENTINEL_OWNERS, msg.sender, exampleCouncilConfig.safeConfig.threshold)
             )
         });
-
-        // Deploy and add the Liveness Module.
-        address livenessModule = deployLivenessModule();
-        vm.stopBroadcast();
-
-        // Since we don't have private keys for the safe owners, we instead use 'startBroadcast' to do something
-        // similar to pranking as the safe. This simulates a quorum of signers executing a transation from the safe to
-        // call it's own 'enableModule' method.
-        vm.startBroadcast(address(safe));
-        safe.enableModule(livenessModule);
-        vm.stopBroadcast();
+        address[] memory owners = safe.getOwners();
+        require(
+            safe.getThreshold() == LivenessModule(livenessModule).getRequiredThreshold(owners.length),
+            "Safe threshold must be equal to the LivenessModule's required threshold"
+        );
         addr_ = address(safe);
         console.log("Deployed and configured the Security Council Safe!");
     }

packages/contracts-bedrock/semver-lock.json

@@ -112,8 +112,8 @@
     "sourceCodeHash": "0xea3872d8f196ae3c863363dfa4b57803cb2a24b0c100244d8f861891e901e03f"
   },
   "src/Safe/LivenessModule.sol": {
-    "initCodeHash": "0xa8b233f0f26f8a73b997b12ba06d64cefa8ee98d523f68cd63320e9787468fae",
-    "sourceCodeHash": "0x15dfd32e92577f4cb5ab05def834a5a1b183e30ca249184f282fca6441be8788"
+    "initCodeHash": "0x6c3d47fdce3dcd5b8499da08061f3ee4ddd126b726bedec601210667bf73db1d",
+    "sourceCodeHash": "0x6bdfa1a9fe8a7bc6e032c2448b911ca3bd55ece0856866042e29a20e270dd0a3"
   },
   "src/cannon/MIPS.sol": {
     "initCodeHash": "0xa5d36fc67170ad87322f358f612695f642757bbf5280800d5d878da21402579a",

packages/contracts-bedrock/src/Safe/LivenessModule.sol

@@ -67,10 +67,6 @@ contract LivenessModule is ISemver {
         MIN_OWNERS = _minOwners;
         address[] memory owners = _safe.getOwners();
         require(_minOwners <= owners.length, "LivenessModule: minOwners must be less than the number of owners");
-        require(
-            _safe.getThreshold() >= getRequiredThreshold(owners.length),
-            "LivenessModule: Insufficient threshold for the number of owners"
-        );
         require(_thresholdPercentage > 0, "LivenessModule: thresholdPercentage must be greater than 0");
         require(_thresholdPercentage <= 100, "LivenessModule: thresholdPercentage must be less than or equal to 100");
     }

packages/contracts-bedrock/test/Safe/DeployOwnership.t.sol

@@ -88,5 +88,8 @@ contract DeployOwnershipTest is Test, DeployOwnership {
         assertEq(LivenessModule(livenessModule).livenessInterval(), lmConfig.livenessInterval);
         assertEq(LivenessModule(livenessModule).thresholdPercentage(), lmConfig.thresholdPercentage);
         assertEq(LivenessModule(livenessModule).minOwners(), lmConfig.minOwners);
+
+        // Ensure the threshold on the safe agrees with the LivenessModule's required threshold
+        assertEq(securityCouncilSafe.getThreshold(), LivenessModule(livenessModule).getRequiredThreshold(owners.length));
     }
 }

packages/contracts-bedrock/test/Safe/LivenessGuard.t.sol

@@ -28,14 +28,20 @@ contract LivenessGuard_TestInit is Test, SafeTestTools {
 
     event OwnerRecorded(address owner);
 
-    uint256 initTime = 10;
     WrappedGuard livenessGuard;
     SafeInstance safeInstance;
 
+    // This needs to be non-zero so that the `lastLive` mapping can record non-zero timestamps
+    uint256 initTime = 10;
+    // These values reflect the planned state of the mainnet Security Council Safe.
+    uint256 threshold = 10;
+    uint256 ownerCount = 13;
+
     /// @dev Sets up the test environment
     function setUp() public {
         vm.warp(initTime);
-        safeInstance = _setupSafe();
+        (, uint256[] memory privKeys) = SafeTestLib.makeAddrsAndKeys("test-owners", ownerCount);
+        safeInstance = _setupSafe(privKeys, threshold);
         livenessGuard = new WrappedGuard(safeInstance.safe);
         safeInstance.setGuard(address(livenessGuard));
     }
@@ -88,8 +94,10 @@ contract LivenessGuard_CheckTx_Test is LivenessGuard_TestInit {
         // Create an array of the addresses who will sign the transaction. SafeTestTools
         // will generate these signatures up to the threshold by iterating over the owners array.
         address[] memory signers = new address[](safeInstance.threshold);
-        signers[0] = safeInstance.owners[0];
-        signers[1] = safeInstance.owners[1];
+        // copy the first threshold owners into the signers array
+        for (uint256 i; i < safeInstance.threshold; i++) {
+            signers[i] = safeInstance.owners[i];
+        }
 
         // Record the timestamps before the transaction
         uint256[] memory beforeTimestamps = new uint256[](safeInstance.owners.length);

packages/contracts-bedrock/test/Safe/LivenessModule.t.sol

@@ -101,23 +101,6 @@ contract LivenessModule_Constructor_TestFail is LivenessModule_TestInit {
             _fallbackOwner: address(0)
         });
     }
-
-    /// @dev Tests that the constructor fails if the minOwners is greater than the number of owners
-    function test_constructor_wrongThreshold_reverts() external {
-        uint256 wrongThreshold = livenessModule.getRequiredThreshold(safeInstance.owners.length) - 1;
-        vm.mockCall(
-            address(safeInstance.safe), abi.encodeCall(OwnerManager.getThreshold, ()), abi.encode(wrongThreshold)
-        );
-        vm.expectRevert("LivenessModule: Insufficient threshold for the number of owners");
-        new LivenessModule({
-            _safe: safeInstance.safe,
-            _livenessGuard: livenessGuard,
-            _livenessInterval: LIVENESS_INTERVAL,
-            _thresholdPercentage: THRESHOLD_PERCENTAGE,
-            _minOwners: MIN_OWNERS,
-            _fallbackOwner: address(0)
-        });
-    }
 }
 
 contract LivenessModule_Getters_Test is LivenessModule_TestInit {