Enable Module Template

src/improvements/doc/NEW_TEMPLATE_GUIDE.md

@@ -33,7 +33,7 @@ function safeAddressString() internal pure override returns (string memory);
 function _taskStorageWrites() internal pure override returns (string[] memory);
 function _templateSetup(string memory taskConfigFilePath) internal override;
 function _build(uint256 chainId) internal override;
-function _validate(uint256 chainId) internal override;
+function _validate(uint256 chainId, VmSafe.AccountAccess[] memory accountAccess) internal override;
 ```
 
 The struct and mapping are optional and can be customized based on task requirements. As a general rule, task developers should think of everything as a template, hence the chainId flag to the build and validate functions.
@@ -138,3 +138,11 @@ parameter2 = 123
 ## Existing Templates
 
 Existing templates can be found in the [`src/improvements/template/`](../template) directory. These templates can be used as a reference for creating new templates.
+
+## Testing
+
+Once a new template has been created and tested from the command line as a forge script, it should be tested in the [regression test suite](../../../test/tasks/Regression.t.sol). Follow the existing examples to add a new test case for the template. 
+
+1. Import the new template into the regression test suite.
+2. Add a new test case that deploys the template and tests its functionality. Make sure to lock the block number and network in the test case to avoid intermittent failures.
+3. Run the regression test suite to ensure the new template passes all tests.

src/improvements/tasks/MultisigTask.sol

@@ -532,7 +532,7 @@ abstract contract MultisigTask is Test, Script {
         AddressRegistry.ChainInfo[] memory chains = addrRegistry.getChains();
 
         for (uint256 i = 0; i < chains.length; i++) {
-            _validate(chains[i].chainId);
+            _validate(chains[i].chainId, accountAccesses);
         }
 
         // check that state diff is as expected
@@ -825,7 +825,8 @@ abstract contract MultisigTask is Test, Script {
     /// @notice task specific validations
     /// @dev override to add additional task specific validations
     /// @param chainId The l2chainId
-    function _validate(uint256 chainId) internal view virtual;
+    /// @param accountAccesses returned from the simulate or execute run function
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory accountAccesses) internal view virtual;
 
     /// @notice validate actions inclusion
     /// default implementation check for duplicate actions
@@ -1010,7 +1011,7 @@ abstract contract MultisigTask is Test, Script {
     /// check the state changes applied by the task. This function can check
     /// that only the nonce changed in the parent multisig when executing a task
     /// by checking the slot and address where the slot changed.
-    function checkStateDiff(VmSafe.AccountAccess[] memory accountAccesses) internal view virtual {
+    function checkStateDiff(VmSafe.AccountAccess[] memory accountAccesses) internal view {
         console.log("Running assertions on the state diff");
         require(accountAccesses.length > 0, "No account accesses");
         address[] memory allowedAccesses = getAllowedStorageAccess();

src/improvements/tasks/OPCMBaseTask.sol

@@ -100,13 +100,13 @@ abstract contract OPCMBaseTask is MultisigTask {
     }
 
     // @notice this function must be overridden in the inheriting contract
-    function _validate(uint256) internal view virtual override {
+    function _validate(uint256, VmSafe.AccountAccess[] memory) internal view virtual override {
         require(false, "You must implement the _validate function");
     }
 
     /// @notice overrides to do nothing per chain
     /// all the chains are handled in a single call to OPCM contract
-    function _buildPerChain(uint256 chainId) internal pure override {
+    function _buildPerChain(uint256) internal pure override {
         // We must override this function but OPCM template do not support per chain builds.
         // We cannot revert here because this build function is called by the parent MultisigTask.
     }

src/improvements/tasks/example/sep/001-enable-deputy-pause-module/README.md

@@ -0,0 +1,15 @@
+# 001-enable-deputy-pause-module
+
+Status: [DRAFT]()
+
+## Objective
+
+This is an example task that should be **deleted** once we have real tasks to run. It's purpose is to have a task that runs before the monorepo-integration-test task.
+
+### Timing
+
+## Transaction creation
+
+## Signing and execution
+
+

src/improvements/tasks/example/sep/001-enable-deputy-pause-module/config.toml

@@ -0,0 +1,7 @@
+# this is the file used to determine the network configuration
+
+l2chains = [{name = "OP Testnet", chainId = 11155420}]
+
+templateName = "EnableDeputyPauseModuleTemplate"
+
+newModule = "0x62f3972c56733aB078F0764d2414DfCaa99d574c"

src/improvements/template/DisputeGameUpgradeTemplate.sol

@@ -2,6 +2,7 @@
 pragma solidity 0.8.15;
 
 import {IDisputeGameFactory, IDisputeGame} from "@eth-optimism-bedrock/interfaces/dispute/IDisputeGameFactory.sol";
+import {VmSafe} from "forge-std/Vm.sol";
 
 import "@eth-optimism-bedrock/src/dispute/lib/Types.sol";
 
@@ -64,7 +65,7 @@ contract DisputeGameUpgradeTemplate is MultisigTask {
 
     /// @notice Validates that implementations were set correctly for the specified chain ID
     /// @param chainId The ID of the L2 chain to validate
-    function _validate(uint256 chainId) internal view override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {
         IDisputeGameFactory disputeGameFactory =
             IDisputeGameFactory(addrRegistry.getAddress("DisputeGameFactoryProxy", chainId));
 

src/improvements/template/EmptyTemplate.template.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+import {VmSafe} from "forge-std/Vm.sol";
+
 import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
 
 /// @notice A template contract for configuring protocol parameters.
@@ -59,7 +61,7 @@ contract EmptyTemplate is MultisigTask {
     }
 
     /// @notice This method performs all validations and assertions that verify the calls executed as expected.
-    function _validate(uint256 chainId) internal pure override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal pure override {
         require(false, "TODO: Implement the logic to validate that the configuration was set as expected.");
         chainId;
     }

src/improvements/template/EnableDeputyPauseModuleTemplate.sol

@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.15;
+
+import {stdStorage, StdStorage} from "forge-std/Test.sol";
+import {IDeputyPauseModule} from "@eth-optimism-bedrock/interfaces/safe/IDeputyPauseModule.sol";
+import {VmSafe} from "forge-std/Vm.sol";
+
+import "forge-std/Test.sol";
+
+import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
+import {ModuleManager} from "lib/safe-contracts/contracts/base/ModuleManager.sol";
+import {AddressRegistry} from "src/improvements/AddressRegistry.sol";
+import {AccountAccessParser} from "src/libraries/AccountAccessParser.sol";
+
+/// @title EnableDeputyPauseModuleTemplate
+/// @notice Template contract for enabling a module in a Gnosis Safe
+contract EnableDeputyPauseModuleTemplate is MultisigTask {
+    using AccountAccessParser for *;
+    using stdStorage for StdStorage;
+
+    /// @notice Module configuration loaded from TOML
+    address public newModule;
+
+    /// @notice Constant safe address string identifier
+    string constant _SAFE_ADDRESS = "FoundationOperationSafe";
+
+    /// @notice Gnosis Safe Sentinel Module address
+    address internal constant SENTINEL_MODULE = address(0x1);
+
+    /// @notice Gnosis Safe Module Mapping Storage Offset
+    uint256 public constant MODULE_MAPPING_STORAGE_OFFSET = 1;
+
+    /// @notice Gnosis Safe Module Nonce Storage Offset
+    bytes32 public constant NONCE_STORAGE_OFFSET = bytes32(uint256(5));
+
+    /// @notice Returns the safe address string identifier
+    /// @return The string "DeputyPauseSafe"
+    function safeAddressString() public pure override returns (string memory) {
+        return _SAFE_ADDRESS;
+    }
+
+    /// @notice Returns the storage write permissions required for this task
+    /// @return Array of storage write permissions
+    function _taskStorageWrites() internal pure override returns (string[] memory) {
+        string[] memory storageWrites;
+
+        storageWrites = new string[](1);
+        storageWrites[0] = _SAFE_ADDRESS;
+
+        return storageWrites;
+    }
+
+    /// @notice Sets up the template with module configuration from a TOML file
+    /// @param taskConfigFilePath Path to the TOML configuration file
+    function _templateSetup(string memory taskConfigFilePath) internal override {
+        string memory file = vm.readFile(taskConfigFilePath);
+        newModule = vm.parseTomlAddress(file, ".newModule");
+        assertNotEq(newModule.code.length, 0, "new module must have code");
+
+        // only allow one chain to be modified at a time with this template
+        AddressRegistry.ChainInfo[] memory _chains =
+            abi.decode(vm.parseToml(vm.readFile(taskConfigFilePath), ".l2chains"), (AddressRegistry.ChainInfo[]));
+
+        assertEq(_chains.length, 1, "Must specify exactly one chain id to enable deputy pause module for");
+    }
+
+    /// @notice Empty implementation as specified
+    /// @param chainId The ID of the L2 chain
+    function _buildPerChain(uint256 chainId) internal override {}
+
+    /// @notice Builds the action for enabling the module in the Safe
+    function _buildSingle() internal override {
+        ModuleManager(parentMultisig).enableModule(newModule);
+    }
+
+    /// @notice Validates that the module was enabled correctly
+    /// @param chainId The ID of the L2 chain to validate
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory accountAccess) internal view override {
+        (address[] memory modules, address nextModule) =
+            ModuleManager(parentMultisig).getModulesPaginated(SENTINEL_MODULE, 100);
+
+        assertTrue(ModuleManager(parentMultisig).isModuleEnabled(newModule), "Module not enabled");
+        assertEq(nextModule, SENTINEL_MODULE, "Next module not correct");
+
+        bool moduleFound;
+        for (uint256 i = 0; i < modules.length; i++) {
+            if (modules[i] == newModule) {
+                moduleFound = true;
+            }
+        }
+        assertTrue(moduleFound, "Module not found in new modules list");
+
+        IDeputyPauseModule deputyGuardianModule = IDeputyPauseModule(newModule);
+        assertEq(deputyGuardianModule.version(), "1.0.0-beta.2", "Deputy Guardian Module version not correct");
+        assertEq(
+            address(deputyGuardianModule.foundationSafe()), parentMultisig, "Deputy Guardian safe pointer not correct"
+        );
+        assertEq(
+            address(deputyGuardianModule.superchainConfig()),
+            addrRegistry.getAddress("SuperchainConfig", chainId),
+            "Superchain config address not correct"
+        );
+
+        bytes32 moduleSlot = keccak256(abi.encode(newModule, MODULE_MAPPING_STORAGE_OFFSET));
+        bytes32 sentinelSlot = keccak256(abi.encode(SENTINEL_MODULE, MODULE_MAPPING_STORAGE_OFFSET));
+
+        bool moduleWriteFound;
+
+        address[] memory uniqueWrites = accountAccess.getUniqueWrites();
+        assertEq(uniqueWrites.length, 1, "should only write to foundation ops safe");
+        assertEq(uniqueWrites[0], parentMultisig, "should only write to foundation ops safe address");
+
+        AccountAccessParser.StateDiff[] memory accountWrites = accountAccess.getStateDiffFor(parentMultisig);
+
+        for (uint256 i = 0; i < accountWrites.length; i++) {
+            AccountAccessParser.StateDiff memory storageAccess = accountWrites[i];
+            assertTrue(
+                storageAccess.slot == NONCE_STORAGE_OFFSET || storageAccess.slot == moduleSlot
+                    || storageAccess.slot == sentinelSlot,
+                "Only nonce and module slot should be updated on upgrade controller multisig"
+            );
+
+            if (storageAccess.slot == moduleSlot) {
+                assertEq(
+                    address(uint160(uint256(storageAccess.newValue))),
+                    modules.length >= 2 ? modules[1] : SENTINEL_MODULE,
+                    "new module not correct"
+                );
+
+                bytes32 sentinelModuleValue = vm.load(parentMultisig, sentinelSlot);
+                assertEq(
+                    sentinelModuleValue, bytes32(uint256(uint160(newModule))), "sentinel does not point to new module"
+                );
+
+                moduleWriteFound = true;
+            }
+        }
+
+        /// module write must be found, else revert
+        assertTrue(moduleWriteFound, "Module write not found");
+    }
+
+    /// @notice No code exceptions for this template
+    function getCodeExceptions() internal view override returns (address[] memory) {}
+}

src/improvements/template/GasConfigTemplate.sol

@@ -2,6 +2,7 @@
 pragma solidity 0.8.15;
 
 import {SystemConfig} from "@eth-optimism-bedrock/src/L1/SystemConfig.sol";
+import {VmSafe} from "forge-std/Vm.sol";
 
 import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
 
@@ -59,7 +60,7 @@ contract GasConfigTemplate is MultisigTask {
 
     /// @notice Validates that gas limits were set correctly for the specified chain ID
     /// @param chainId The ID of the L2 chain to validate
-    function _validate(uint256 chainId) internal view override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {
         SystemConfig systemConfig = SystemConfig(addrRegistry.getAddress("SystemConfigProxy", chainId));
 
         if (gasLimits[chainId] != 0) {

src/improvements/template/SetGameTypeTemplate.sol

@@ -5,6 +5,7 @@ import {
     DeputyGuardianModule, IOptimismPortal2, GameType
 } from "@eth-optimism-bedrock/src/safe/DeputyGuardianModule.sol";
 import {LibGameType} from "@eth-optimism-bedrock/src/dispute/lib/LibUDT.sol";
+import {VmSafe} from "forge-std/Vm.sol";
 
 import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
 
@@ -67,7 +68,7 @@ contract SetGameTypeTemplate is MultisigTask {
 
     /// @notice Validates that game types were set correctly for the specified chain ID
     /// @param chainId The ID of the L2 chain to validate
-    function _validate(uint256 chainId) internal view override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {
         IOptimismPortal2 optimismPortal =
             IOptimismPortal2(payable(addrRegistry.getAddress("OptimismPortalProxy", chainId)));
 

src/improvements/template/TestOPCMUpgradeVxyz.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.15;
 
+import {VmSafe} from "forge-std/Vm.sol";
+
 import {OPCMBaseTask} from "../tasks/OPCMBaseTask.sol";
 import {AddressRegistry} from "src/improvements/AddressRegistry.sol";
 
@@ -74,7 +76,7 @@ contract TestOPCMUpgradeVxyz is OPCMBaseTask {
     /// @notice validate the task for a given l2chain
     /// for this dummy opcm there are no validations per l2 chain
     /// for a real OPCM instance, add the validations per l2chain
-    function _validate(uint256 chainId) internal view override {}
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {}
 
     /// @notice no code exceptions for this template
     function getCodeExceptions() internal view virtual override returns (address[] memory) {}

src/improvements/template/TransferOwnerTemplate.sol

@@ -2,6 +2,7 @@
 pragma solidity 0.8.15;
 
 import {ProxyAdmin} from "@eth-optimism-bedrock/src/universal/ProxyAdmin.sol";
+import {VmSafe} from "forge-std/Vm.sol";
 
 import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
 import {AddressRegistry} from "src/improvements/AddressRegistry.sol";
@@ -48,7 +49,7 @@ contract TransferOwnerTemplate is MultisigTask {
 
     /// @notice Validates that gas limits were set correctly for the specified chain ID
     /// @param chainId The ID of the L2 chain to validate
-    function _validate(uint256 chainId) internal view override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {
         ProxyAdmin proxyAdmin = ProxyAdmin(addrRegistry.getAddress("ProxyAdmin", chainId));
 
         assertEq(proxyAdmin.owner(), newOwner, "new owner not set correctly");

test/tasks/Regression.t.sol

@@ -7,8 +7,9 @@ import {IGnosisSafe} from "@base-contracts/script/universal/IGnosisSafe.sol";
 import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol";
 import {GasConfigTemplate} from "src/improvements/template/GasConfigTemplate.sol";
 import {SetGameTypeTemplate} from "src/improvements/template/SetGameTypeTemplate.sol";
-import {DisputeGameUpgradeTemplate} from "src/improvements/template/DisputeGameUpgradeTemplate.sol";
 import {TestOPCMUpgradeVxyz} from "src/improvements/template/TestOPCMUpgradeVxyz.sol";
+import {DisputeGameUpgradeTemplate} from "src/improvements/template/DisputeGameUpgradeTemplate.sol";
+import {EnableDeputyPauseModuleTemplate} from "src/improvements/template/EnableDeputyPauseModuleTemplate.sol";
 
 /// @notice test that the call data and data to sign generated in simulateRun for the multisigs
 /// are always the same. This means that if there were any bug introduced in the multisig task, or opcm base task,
@@ -110,6 +111,24 @@ contract RegressionTest is Test {
         assertEq(keccak256(bytes(dataToSign)), keccak256(bytes(expectedDataToSign)));
     }
 
+    function testRegressionCallDataMatches_EnableDeputyPauseModuleTemplate() public {
+        string memory taskConfigFilePath = "test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml";
+        string memory expectedCallData =
+            "0x174dea71000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000837de453ad5f21e89771e3c06239d8236c0efd5e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000024610b592500000000000000000000000062f3972c56733ab078f0764d2414dfcaa99d574c00000000000000000000000000000000000000000000000000000000";
+        vm.createSelectFork("sepolia", 7745524);
+        MultisigTask multisigTask = new EnableDeputyPauseModuleTemplate();
+        multisigTask.simulateRun(taskConfigFilePath);
+
+        string memory callData = vm.toString(multisigTask.getCalldata());
+        assertEq(keccak256(bytes(callData)), keccak256(bytes(expectedCallData)));
+
+        string memory expectedDataToSign =
+            "0x1901e84ad8db37faa1651b140c17c70e4c48eaa47a635e0db097ddf4ce1cc14b9ecbf55e2ed894ddff4c0045537c8239db1c4b3ac5700049164b5823ecaa045d7334";
+        string memory dataToSign =
+            vm.toString(multisigTask.getDataToSign(multisigTask.parentMultisig(), multisigTask.getCalldata()));
+        assertEq(keccak256(bytes(dataToSign)), keccak256(bytes(expectedDataToSign)));
+    }
+
     function testRegressionCallDataMatches_OPCMUpgradeVxyz() public {
         string memory taskConfigFilePath = "test/tasks/mock/configs/TestOPCMUpgradeVxyz.toml";
         // call data generated by manually running the TestOPCMUpgradeVxyz template at block 7733622 on sepolia

test/tasks/mock/MockMultisigTask.sol

@@ -49,7 +49,7 @@ contract MockMultisigTask is MultisigTask {
         }
     }
 
-    function _validate(uint256 chainId) internal view override {
+    function _validate(uint256 chainId, VmSafe.AccountAccess[] memory) internal view override {
         IProxy proxy = IProxy(payable(addrRegistry.getAddress("L1ERC721BridgeProxy", chainId)));
         bytes32 data = vm.load(address(proxy), Constants.PROXY_IMPLEMENTATION_ADDRESS);
 

test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml

@@ -0,0 +1,7 @@
+# this is the file used to determine the network configuration
+
+l2chains = [{name = "OP Testnet", chainId = 11155420}]
+
+templateName = "EnableDeputyPauseModuleTemplate"
+
+newModule = "0x62f3972c56733aB078F0764d2414DfCaa99d574c"