diff --git a/packages/contracts-bedrock/scripts/Deploy.s.sol b/packages/contracts-bedrock/scripts/Deploy.s.sol index 18cb83d9b8bb..7b59c3c82b92 100644 --- a/packages/contracts-bedrock/scripts/Deploy.s.sol +++ b/packages/contracts-bedrock/scripts/Deploy.s.sol @@ -8,6 +8,7 @@ import { console2 as console } from "forge-std/console2.sol"; import { stdJson } from "forge-std/StdJson.sol"; import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol"; +import { OwnerManager } from "safe-contracts/base/OwnerManager.sol"; import { GnosisSafeProxyFactory as SafeProxyFactory } from "safe-contracts/proxies/GnosisSafeProxyFactory.sol"; import { Enum as SafeOps } from "safe-contracts/common/Enum.sol"; @@ -426,6 +427,13 @@ contract Deploy is Deployer { addr_ = deploySafe(_name, owners, 1, true); } + /// @notice Deploy a new Safe contract. If the keepDeployer option is used to enable further setup actions, then + /// the removeDeployerFromSafe() function should be called on that safe after setup is complete. + /// Note this function does not have the broadcast modifier. + /// @param _name The name of the Safe to deploy. + /// @param _owners The owners of the Safe. + /// @param _threshold The threshold of the Safe. + /// @param _keepDeployer Wether or not the deployer address will be added as an owner of the Safe. function deploySafe( string memory _name, address[] memory _owners, @@ -462,6 +470,24 @@ contract Deploy is Deployer { console.log("New safe: %s deployed at %s\n Note that this safe is owned by the deployer key", _name, addr_); } + /// @notice If the keepDeployer option was used with deploySafe(), this function can be used to remove the deployer. + /// Note this function does not have the broadcast modifier. + function removeDeployerFromSafe(string memory _name, uint256 _newThreshold) public { + Safe safe = Safe(mustGetAddress(_name)); + + // The sentinel address is used to mark the start and end of the linked list of owners in the Safe. + address sentinelOwners = address(0x1); + + // Because deploySafe() always adds msg.sender first (if keepDeployer is true), we know that the previousOwner + // will be sentinelOwners. + _callViaSafe({ + _safe: safe, + _target: address(safe), + _data: abi.encodeCall(OwnerManager.removeOwner, (sentinelOwners, msg.sender, _newThreshold)) + }); + console.log("Removed deployer owner from ", _name); + } + /// @notice Deploy the AddressManager function deployAddressManager() public broadcast returns (address addr_) { console.log("Deploying AddressManager"); diff --git a/packages/contracts-bedrock/scripts/DeployOwnership.s.sol b/packages/contracts-bedrock/scripts/DeployOwnership.s.sol index 1bc866ea1137..3edfa4d70bcd 100644 --- a/packages/contracts-bedrock/scripts/DeployOwnership.s.sol +++ b/packages/contracts-bedrock/scripts/DeployOwnership.s.sol @@ -50,9 +50,6 @@ struct GuardianConfig { DeputyGuardianModuleConfig deputyGuardianModuleConfig; } -// The sentinel address is used to mark the start and end of the linked list of owners in the Safe. -address constant SENTINEL_OWNERS = address(0x1); - /// @title Deploy /// @notice Script used to deploy and configure the Safe contracts which are used to manage the Superchain, /// as the ProxyAdminOwner and other roles in the system. Note that this script is not executable in a @@ -84,7 +81,7 @@ contract DeployOwnership is Deploy { } /// @notice Returns a GuardianConfig similar to that of the Guardian Safe on Mainnet. - function _getExampleGuardianConfig() internal returns (GuardianConfig memory guardianConfig_) { + function _getExampleGuardianConfig() internal view returns (GuardianConfig memory guardianConfig_) { address[] memory exampleGuardianOwners = new address[](1); exampleGuardianOwners[0] = mustGetAddress("SecurityCouncilSafe"); guardianConfig_ = GuardianConfig({ @@ -161,7 +158,6 @@ contract DeployOwnership is Deploy { /// @notice Deploy a DeputyGuardianModule for use on the Security Council Safe. /// Note this function does not have the broadcast modifier. function deployDeputyGuardianModule() public returns (address addr_) { - SecurityCouncilConfig memory councilConfig = _getExampleCouncilConfig(); Safe councilSafe = Safe(payable(mustGetAddress("SecurityCouncilSafe"))); DeputyGuardianModuleConfig memory deputyGuardianModuleConfig = _getExampleGuardianConfig().deputyGuardianModuleConfig; @@ -203,23 +199,16 @@ contract DeployOwnership is Deploy { /// @notice Configure the Guardian Safe with the DeputyGuardianModule. function configureGuardianSafe() public broadcast returns (address addr_) { - Safe safe = Safe(payable(mustGetAddress("GuardianSafe"))); + addr_ = mustGetAddress("GuardianSafe"); address deputyGuardianModule = deployDeputyGuardianModule(); _callViaSafe({ - _safe: safe, - _target: address(safe), + _safe: Safe(payable(addr_)), + _target: addr_, _data: abi.encodeCall(ModuleManager.enableModule, (deputyGuardianModule)) }); - // 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 - // will be SENTINEL_OWNERS. - _callViaSafe({ - _safe: safe, - _target: address(safe), - _data: abi.encodeCall(OwnerManager.removeOwner, (SENTINEL_OWNERS, msg.sender, 1)) - }); + // Finalize configuration by removing the additional deployer key. + removeDeployerFromSafe({ _name: "GuardianSafe", _newThreshold: 1 }); console.log("DeputyGuardianModule enabled on GuardianSafe"); } @@ -242,22 +231,15 @@ contract DeployOwnership is Deploy { _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 - // will be SENTINEL_OWNERS. - _callViaSafe({ - _safe: safe, - _target: address(safe), - _data: abi.encodeCall( - OwnerManager.removeOwner, (SENTINEL_OWNERS, msg.sender, exampleCouncilConfig.safeConfig.threshold) - ) - }); + // Finalize configuration by removing the additional deployer key. + removeDeployerFromSafe({ _name: "SecurityCouncilSafe", _newThreshold: exampleCouncilConfig.safeConfig.threshold }); + 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!"); } diff --git a/packages/contracts-bedrock/test/Safe/DeployOwnership.t.sol b/packages/contracts-bedrock/test/Safe/DeployOwnership.t.sol index a382b8f2cd7c..65436e4c28a4 100644 --- a/packages/contracts-bedrock/test/Safe/DeployOwnership.t.sol +++ b/packages/contracts-bedrock/test/Safe/DeployOwnership.t.sol @@ -36,6 +36,7 @@ contract DeployOwnershipTest is Test, DeployOwnership { address[] memory safeOwners = _safe.getOwners(); assertEq(_safeConfig.owners.length, safeOwners.length); + assertFalse(_safe.isOwner(msg.sender)); for (uint256 i = 0; i < safeOwners.length; i++) { assertEq(safeOwners[i], _safeConfig.owners[i]); }