diff --git a/lib/superchain-registry b/lib/superchain-registry index fb6f538e17..08e3fe429c 160000 --- a/lib/superchain-registry +++ b/lib/superchain-registry @@ -1 +1 @@ -Subproject commit fb6f538e17ee296b19536b03b8c73adc6041c60d +Subproject commit 08e3fe429c776a532c2b6dc09571fc13e6dba5d4 diff --git a/src/improvements/SimpleAddressRegistry.sol b/src/improvements/SimpleAddressRegistry.sol index 860351fef7..b978852f4d 100644 --- a/src/improvements/SimpleAddressRegistry.sol +++ b/src/improvements/SimpleAddressRegistry.sol @@ -22,6 +22,13 @@ contract SimpleAddressRegistry is StdChains { /// @notice Initializes the contract by loading addresses from the TOML config file. constructor(string memory _configPath) { + string memory chainKey; + if (block.chainid == getChain("mainnet").chainId) chainKey = ".eth"; + else if (block.chainid == getChain("sepolia").chainId) chainKey = ".sep"; + else if (block.chainid == getChain("optimism").chainId) chainKey = ".oeth"; + + if (bytes(chainKey).length > 0) _loadHardcodedAddresses(chainKey); + string memory toml = vm.readFile(_configPath); if (!toml.keyExists(".addresses")) return; // If the addresses section is missing, do nothing. @@ -32,13 +39,6 @@ contract SimpleAddressRegistry is StdChains { _registerAddress(key, who); } - - string memory chainKey; - if (block.chainid == getChain("mainnet").chainId) chainKey = ".eth"; - else if (block.chainid == getChain("sepolia").chainId) chainKey = ".sep"; - else if (block.chainid == getChain("optimism").chainId) chainKey = ".oeth"; - - if (bytes(chainKey).length > 0) _loadHardcodedAddresses(chainKey); } /// @notice Retrieves an address by its contract identifier. diff --git a/src/improvements/addresses.toml b/src/improvements/addresses.toml index 45ed6887be..8a6724c1d9 100644 --- a/src/improvements/addresses.toml +++ b/src/improvements/addresses.toml @@ -3,10 +3,12 @@ ChainGovernorSafe = "0xb0c4C487C5cf6d67807Bc2008c66fa7e2cE744EC" FoundationOperationSafe = "0x9BA6e03D8B90dE867373Db8cF1A58d2F7F006b3A" FoundationUpgradeSafe = "0x847B5c174615B1B7fDF770882256e2D3E95b9D92" SecurityCouncil = "0xc2819DC788505Aac350142A7A707BF9D03E3Bd03" +SuperchainConfig = "0x95703e0982140D16f8ebA6d158FccEde42f04a4C" [sep] FoundationOperationSafe = "0x837DE453AD5F21E89771e3c06239d8236c0EFd5E" FoundationUpgradeSafe = "0xDEe57160aAfCF04c34C887B5962D0a69676d3C8B" SecurityCouncil = "0xf64bc17485f0B4Ea5F06A96514182FC4cB561977" +SuperchainConfig = "0xC2Be75506d5724086DEB7245bd260Cc9753911Be" [oeth] \ No newline at end of file diff --git a/src/improvements/script/get-rpc-url.sh b/src/improvements/script/get-rpc-url.sh index 3aa11ee604..baba92ff0f 100755 --- a/src/improvements/script/get-rpc-url.sh +++ b/src/improvements/script/get-rpc-url.sh @@ -8,7 +8,9 @@ if [[ "$TASK_PATH" == *"/eth/"* ]]; then echo "mainnet" elif [[ "$TASK_PATH" == *"/sep/"* ]]; then echo "sepolia" +elif [[ "$TASK_PATH" == *"/oeth/"* ]]; then + echo "opMainnet" else - echo "Error: Task path must contain either /eth/ or /sep/" >&2 + echo "Error: Task path must contain either /eth/ or /sep/ or /oeth/" >&2 exit 1 fi diff --git a/src/improvements/tasks/MultisigTask.sol b/src/improvements/tasks/MultisigTask.sol index 09a55d004e..40e5e5b9e7 100644 --- a/src/improvements/tasks/MultisigTask.sol +++ b/src/improvements/tasks/MultisigTask.sol @@ -6,17 +6,20 @@ import {console} from "forge-std/console.sol"; import {Script} from "forge-std/Script.sol"; import {VmSafe} from "forge-std/Vm.sol"; import {Test} from "forge-std/Test.sol"; +import {stdToml} from "forge-std/StdToml.sol"; import {Signatures} from "@base-contracts/script/universal/Signatures.sol"; import {Simulation} from "@base-contracts/script/universal/Simulation.sol"; import {IGnosisSafe, Enum} from "@base-contracts/script/universal/IGnosisSafe.sol"; +import {SimpleAddressRegistry} from "src/improvements/SimpleAddressRegistry.sol"; import {SuperchainAddressRegistry} from "src/improvements/SuperchainAddressRegistry.sol"; import {AccountAccessParser} from "src/libraries/AccountAccessParser.sol"; +import {StateOverrideManager} from "src/improvements/tasks/StateOverrideManager.sol"; type AddressRegistry is address; -abstract contract MultisigTask is Test, Script { +abstract contract MultisigTask is Test, Script, StateOverrideManager { using EnumerableSet for EnumerableSet.AddressSet; using AccountAccessParser for VmSafe.AccountAccess[]; @@ -72,6 +75,12 @@ abstract contract MultisigTask is Test, Script { bytes32 newValue; } + /// @notice Enum to determine the type of task + enum TaskType { + L2TaskBase, + SimpleBase + } + /// @notice transfers during task execution mapping(address => TransferInfo[]) private _taskTransfers; @@ -122,6 +131,9 @@ abstract contract MultisigTask is Test, Script { // ================================================== // These are functions have no default implementation and MUST be implemented by the inheriting contract. + /// @notice Returns the type of task. L2TaskBase or SimpleBase. + function taskType() public pure virtual returns (TaskType); + /// @notice Specifies the safe address string to run the template from. This string refers /// to a named contract, where the name is read from an address registry contract. function safeAddressString() public pure virtual returns (string memory); @@ -175,6 +187,9 @@ abstract contract MultisigTask is Test, Script { // sets safe to the safe specified by the current template from addresses.json _taskSetup(taskConfigFilePath); + // Overrides only get applied when simulating + _overrideState(taskConfigFilePath); + // now execute task actions Action[] memory actions = build(); VmSafe.AccountAccess[] memory accountAccesses = simulate(signatures, actions); @@ -271,11 +286,12 @@ abstract contract MultisigTask is Test, Script { IGnosisSafe _parentMultisig; // TODO parentMultisig should be of type IGnosisSafe (addrRegistry, _parentMultisig, multicallTarget) = _configureTask(taskConfigFilePath); + parentMultisig = address(_parentMultisig); _templateSetup(taskConfigFilePath); + nonce = IGnosisSafe(parentMultisig).nonce(); // Maybe be overridden later by state overrides - nonce = IGnosisSafe(parentMultisig).nonce(); // TODO change this once we implement task stacking startingOwners = IGnosisSafe(parentMultisig).getOwners(); vm.label(AddressRegistry.unwrap(addrRegistry), "AddrRegistry"); @@ -370,9 +386,7 @@ abstract contract MultisigTask is Test, Script { // Execute the transaction execTransaction(parentMultisig, multicallTarget, 0, callData, Enum.Operation.DelegateCall, signatures); - VmSafe.AccountAccess[] memory accountAccesses = vm.stopAndReturnStateDiff(); - return accountAccesses; } @@ -660,13 +674,19 @@ abstract contract MultisigTask is Test, Script { /// @notice print the tenderly simulation link with the state overrides function printTenderlySimulationLink(Action[] memory actions) internal view { - Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](1); - overrides[0] = - Simulation.overrideSafeThresholdOwnerAndNonce(parentMultisig, msg.sender, _getNonce(parentMultisig)); + Simulation.StateOverride[] memory allStateOverrides = + getStateOverrides(parentMultisig, _getNonce(parentMultisig)); + bytes memory txData = _execTransationCalldata( parentMultisig, getMulticall3Calldata(actions), Signatures.genPrevalidatedSignature(msg.sender) ); - Simulation.logSimulationLink({_to: parentMultisig, _data: txData, _from: msg.sender, _overrides: overrides}); + + Simulation.logSimulationLink({ + _to: parentMultisig, + _data: txData, + _from: msg.sender, + _overrides: allStateOverrides + }); } /// @notice get the hash for this safe transaction @@ -874,6 +894,12 @@ abstract contract MultisigTask is Test, Script { return validActions; } + /// @notice Override the state of the task. Function is called only when simulating. + function _overrideState(string memory taskConfigFilePath) private { + _applyStateOverrides(taskConfigFilePath); + nonce = _getNonceOrOverride(address(parentMultisig)); + } + /// @dev Returns true if the given account access should be recorded as an action. function _isValidAction(VmSafe.AccountAccess memory access, uint256 topLevelDepth) internal view returns (bool) { bool accountNotRegistryOrVm = @@ -1053,6 +1079,15 @@ abstract contract L2TaskBase is MultisigTask { SuperchainAddressRegistry public superchainAddrRegistry; + /// @notice Returns the type of task. L2TaskBase. + /// Overrides the taskType function in the MultisigTask contract. + function taskType() public pure override returns (TaskType) { + return TaskType.L2TaskBase; + } + + /// @notice Configures the task for L2TaskBase type tasks. + /// Overrides the configureTask function in the MultisigTask contract. + /// For L2TaskBase, we need to configure the superchain address registry. function _configureTask(string memory taskConfigFilePath) internal virtual @@ -1065,28 +1100,20 @@ abstract contract L2TaskBase is MultisigTask { addrRegistry_ = AddressRegistry.wrap(address(superchainAddrRegistry)); SuperchainAddressRegistry.ChainInfo[] memory chains = superchainAddrRegistry.getChains(); - // TODO: This is a hack to support the EnableDeputyPauseModuleTemplate. - // remove this once we have SimpleTaskBase contract as then this template will derive from SimpleTaskBase - // instead of L2TaskBase. - if (keccak256(bytes(config.safeAddressString)) == keccak256(bytes("FoundationOperationSafe"))) { - console.log("Using FoundationOperationSafe"); - parentMultisig_ = IGnosisSafe(superchainAddrRegistry.get(config.safeAddressString)); - } else { - parentMultisig_ = - IGnosisSafe(superchainAddrRegistry.getAddress(config.safeAddressString, chains[0].chainId)); - // Ensure that all chains have the same parentMultisig. - for (uint256 i = 1; i < chains.length; i++) { - require( - address(parentMultisig_) - == superchainAddrRegistry.getAddress(config.safeAddressString, chains[i].chainId), - string.concat( - "MultisigTask: safe address mismatch. Caller: ", - getAddressLabel(address(parentMultisig_)), - ". Actual address: ", - getAddressLabel(superchainAddrRegistry.getAddress(config.safeAddressString, chains[i].chainId)) - ) - ); - } + + parentMultisig_ = IGnosisSafe(superchainAddrRegistry.getAddress(config.safeAddressString, chains[0].chainId)); + // Ensure that all chains have the same parentMultisig. + for (uint256 i = 1; i < chains.length; i++) { + require( + address(parentMultisig_) + == superchainAddrRegistry.getAddress(config.safeAddressString, chains[i].chainId), + string.concat( + "MultisigTask: safe address mismatch. Caller: ", + getAddressLabel(address(parentMultisig_)), + ". Actual address: ", + getAddressLabel(superchainAddrRegistry.getAddress(config.safeAddressString, chains[i].chainId)) + ) + ); } console.log("Parent multisig: ", address(parentMultisig_)); @@ -1097,19 +1124,49 @@ abstract contract L2TaskBase is MultisigTask { // update the config to include the addresses whose storage slots changed, // or figure out why the storage slots are being changed when they should not be. for (uint256 i = 0; i < config.allowedStorageWriteAccesses.length; i++) { - // TODO: This is a hack to support the EnableDeputyPauseModuleTemplate. - // remove this once we have SimpleTaskBase contract as then this template will derive from SimpleTaskBase - // instead of L2TaskBase. - if (keccak256(bytes(config.allowedStorageWriteAccesses[i])) == keccak256(bytes("FoundationOperationSafe"))) - { - _allowedStorageAccesses.add(superchainAddrRegistry.get(config.allowedStorageWriteAccesses[i])); - } else { - for (uint256 j = 0; j < chains.length; j++) { - _allowedStorageAccesses.add( - superchainAddrRegistry.getAddress(config.allowedStorageWriteAccesses[i], chains[j].chainId) - ); - } + for (uint256 j = 0; j < chains.length; j++) { + _allowedStorageAccesses.add( + superchainAddrRegistry.getAddress(config.allowedStorageWriteAccesses[i], chains[j].chainId) + ); } } } } + +abstract contract SimpleBase is MultisigTask { + using EnumerableSet for EnumerableSet.AddressSet; + + SimpleAddressRegistry public simpleAddrRegistry; + + /// @notice Returns the type of task. SimpleBase. + /// Overrides the taskType function in the MultisigTask contract. + function taskType() public pure override returns (TaskType) { + return TaskType.SimpleBase; + } + + /// @notice Configures the task for SimpleBase type tasks. + /// Overrides the configureTask function in the MultisigTask contract. + /// For SimpleBase, we need to configure the simple address registry. + function _configureTask(string memory taskConfigFilePath) + internal + virtual + override + returns (AddressRegistry addrRegistry_, IGnosisSafe parentMultisig_, address multicallTarget_) + { + multicallTarget_ = MULTICALL3_ADDRESS; + + simpleAddrRegistry = new SimpleAddressRegistry(taskConfigFilePath); + addrRegistry_ = AddressRegistry.wrap(address(simpleAddrRegistry)); + + parentMultisig_ = IGnosisSafe(simpleAddrRegistry.get(config.safeAddressString)); + + // This loads the allowed storage write accesses to storage for this task. + // If this task changes storage slots outside of the allowed write accesses, + // then the task will fail at runtime and the task developer will need to + // update the config to include the addresses whose storage slots changed, + // or figure out why the storage slots are being changed when they should not be. + for (uint256 i = 0; i < config.allowedStorageWriteAccesses.length; i++) { + _allowedStorageAccesses.add(simpleAddrRegistry.get(config.allowedStorageWriteAccesses[i])); + } + } +} diff --git a/src/improvements/tasks/StateOverrideManager.sol b/src/improvements/tasks/StateOverrideManager.sol new file mode 100644 index 0000000000..b32f3f12dd --- /dev/null +++ b/src/improvements/tasks/StateOverrideManager.sol @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import {Vm} from "forge-std/Vm.sol"; +import {stdToml} from "forge-std/StdToml.sol"; +import {Simulation} from "@base-contracts/script/universal/Simulation.sol"; +import {IGnosisSafe} from "@base-contracts/script/universal/IGnosisSafe.sol"; + +contract StateOverrideManager { + using stdToml for string; + + address private constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); + Vm private constant vm = Vm(VM_ADDRESS); + + /// @notice The state overrides for the local and tenderly simulation + Simulation.StateOverride[] internal _stateOverrides; + + function getStateOverrides(address parentMultisig, uint256 parentMultisigNonce) + public + view + returns (Simulation.StateOverride[] memory) + { + // Append user defined overrides to the default tenderly overrides. + // This means that the user defined overrides take precedence over the default tenderly overrides. + Simulation.StateOverride memory defaultOverride = + _createDefaultTenderlyOverride(parentMultisig, parentMultisigNonce); + Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](1 + _stateOverrides.length); + overrides[0] = defaultOverride; + for (uint256 i = 0; i < _stateOverrides.length; i++) { + overrides[i + 1] = _stateOverrides[i]; + } + return overrides; + } + + /// @notice This function must be called first before any other function that uses state overrides. + function _applyStateOverrides(string memory taskConfigFilePath) internal { + _readStateOverrides(taskConfigFilePath); + for (uint256 i = 0; i < _stateOverrides.length; i++) { + for (uint256 j = 0; j < _stateOverrides[i].overrides.length; j++) { + vm.store( + address(_stateOverrides[i].contractAddress), + _stateOverrides[i].overrides[j].key, + _stateOverrides[i].overrides[j].value + ); + } + } + } + + /// @notice Creates a default state override for the parent multisig (nonce, threshold, owner). + function _createDefaultTenderlyOverride(address parentMultisig, uint256 nonce) + internal + view + returns (Simulation.StateOverride memory) + { + Simulation.StateOverride memory defaultOverride; + defaultOverride.contractAddress = parentMultisig; + defaultOverride = Simulation.addOverride( + defaultOverride, Simulation.StorageOverride({key: bytes32(uint256(0x4)), value: bytes32(uint256(0x1))}) + ); + defaultOverride = Simulation.addOverride( + defaultOverride, Simulation.StorageOverride({key: bytes32(uint256(0x5)), value: bytes32(nonce)}) + ); + defaultOverride = Simulation.addOwnerOverride(parentMultisig, defaultOverride, msg.sender); + return defaultOverride; + } + + function _readStateOverrides(string memory taskConfigFilePath) private { + string memory toml = vm.readFile(taskConfigFilePath); + string memory stateOverridesKey = ".stateOverrides"; + if (!toml.keyExists(stateOverridesKey)) return; + + string[] memory targetsStrs = vm.parseTomlKeys(toml, stateOverridesKey); + Simulation.StateOverride[] memory stateOverridesMemory = new Simulation.StateOverride[](targetsStrs.length); + + address[] memory targetsAddrs = new address[](targetsStrs.length); + for (uint256 i = 0; i < targetsStrs.length; i++) { + targetsAddrs[i] = vm.parseAddress(targetsStrs[i]); + } + for (uint256 i = 0; i < targetsAddrs.length; i++) { + Simulation.StorageOverride[] memory overrides = abi.decode( + vm.parseToml(toml, string.concat(stateOverridesKey, ".", targetsStrs[i])), + (Simulation.StorageOverride[]) + ); + stateOverridesMemory[i] = Simulation.StateOverride({contractAddress: targetsAddrs[i], overrides: overrides}); + } + // Cannot assign the abi.decode result to `_stateOverrides` directly because it's a storage array, so + // compiling without via-ir will fail with: + // Unimplemented feature (/solidity/libsolidity/codegen/ArrayUtils.cpp:228):Copying of type struct Simulation.StateOverride memory[] memory to storage not yet supported. + for (uint256 i = 0; i < stateOverridesMemory.length; i++) { + // Push a new element into the storage array and get a reference to it. + Simulation.StateOverride storage stateOverrideStorage = _stateOverrides.push(); + stateOverrideStorage.contractAddress = stateOverridesMemory[i].contractAddress; + for (uint256 j = 0; j < stateOverridesMemory[i].overrides.length; j++) { + stateOverrideStorage.overrides.push(stateOverridesMemory[i].overrides[j]); + } + } + } + + function _getNonceOrOverride(address parentMultisig) internal view returns (uint256 nonce_) { + bool foundNonceOverride = false; + for (uint256 i = 0; i < _stateOverrides.length; i++) { + bytes32 GNOSIS_SAFE_NONCE_SLOT = bytes32(uint256(0x5)); + for (uint256 j = 0; j < _stateOverrides[i].overrides.length; j++) { + if ( + _stateOverrides[i].contractAddress == parentMultisig + && _stateOverrides[i].overrides[j].key == GNOSIS_SAFE_NONCE_SLOT + ) { + foundNonceOverride = true; + nonce_ = uint256(_stateOverrides[i].overrides[j].value); + } + } + } + if (!foundNonceOverride) { + nonce_ = IGnosisSafe(parentMultisig).nonce(); + } + } +} diff --git a/src/improvements/tasks/TaskRunner.sol b/src/improvements/tasks/TaskRunner.sol index af673e1bc5..2aa7bf2738 100644 --- a/src/improvements/tasks/TaskRunner.sol +++ b/src/improvements/tasks/TaskRunner.sol @@ -6,6 +6,7 @@ import {Script} from "forge-std/Script.sol"; import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol"; import {SuperchainAddressRegistry} from "src/improvements/SuperchainAddressRegistry.sol"; +import {SimpleAddressRegistry} from "src/improvements/SimpleAddressRegistry.sol"; /// This script gathers all tasks for a given network and performs a simulation run for each task. /// Once all tasks are simulated, the resultant state is written to a file. @@ -76,18 +77,19 @@ contract TaskRunner is Script { string memory templatePath = string.concat("out/", templateName, ".sol/", templateName, ".json"); MultisigTask task = MultisigTask(deployCode(templatePath)); string memory safeAddressString = task.safeAddressString(); + MultisigTask.TaskType taskType = task.taskType(); - SuperchainAddressRegistry _addrRegistry = new SuperchainAddressRegistry(taskConfigFilePath); - SuperchainAddressRegistry.ChainInfo[] memory chains = _addrRegistry.getChains(); address parentMultisig; - // TODO: This is a hack to support the EnableDeputyPauseModuleTemplate. - // remove this once we have SimpleTaskBase contract as then this template will derive from SimpleTaskBase - // instead of L2TaskBase. - if (keccak256(bytes(safeAddressString)) == keccak256(bytes("FoundationOperationSafe"))) { - parentMultisig = _addrRegistry.get(safeAddressString); + + if (taskType == MultisigTask.TaskType.SimpleBase) { + SimpleAddressRegistry _simpleAddrRegistry = new SimpleAddressRegistry(taskConfigFilePath); + parentMultisig = _simpleAddrRegistry.get(safeAddressString); } else { + SuperchainAddressRegistry _addrRegistry = new SuperchainAddressRegistry(taskConfigFilePath); + SuperchainAddressRegistry.ChainInfo[] memory chains = _addrRegistry.getChains(); parentMultisig = _addrRegistry.getAddress(safeAddressString, chains[0].chainId); } + return task.isNestedSafe(parentMultisig); } } diff --git a/src/improvements/tasks/example/sep/001-enable-deputy-pause-module/config.toml b/src/improvements/tasks/example/sep/001-enable-deputy-pause-module/config.toml index d346e93830..148e72457c 100644 --- a/src/improvements/tasks/example/sep/001-enable-deputy-pause-module/config.toml +++ b/src/improvements/tasks/example/sep/001-enable-deputy-pause-module/config.toml @@ -1,7 +1,4 @@ # this is the file used to determine the network configuration - -l2chains = [{name = "OP Testnet", chainId = 11155420}] - templateName = "EnableDeputyPauseModuleTemplate" newModule = "0x62f3972c56733aB078F0764d2414DfCaa99d574c" diff --git a/src/improvements/template/EmptyTemplate.template.sol b/src/improvements/template/EmptyTemplate.template.sol index 132336261e..629ef2c1c4 100644 --- a/src/improvements/template/EmptyTemplate.template.sol +++ b/src/improvements/template/EmptyTemplate.template.sol @@ -77,4 +77,8 @@ contract EmptyTemplate is MultisigTask { codeExceptions[0] = address(0); return codeExceptions; } + + function taskType() public pure override returns (TaskType) { + return TaskType.SimpleBase; + } } diff --git a/src/improvements/template/EnableDeputyPauseModuleTemplate.sol b/src/improvements/template/EnableDeputyPauseModuleTemplate.sol index 285eca5419..f5a9504231 100644 --- a/src/improvements/template/EnableDeputyPauseModuleTemplate.sol +++ b/src/improvements/template/EnableDeputyPauseModuleTemplate.sol @@ -7,13 +7,13 @@ import {VmSafe} from "forge-std/Vm.sol"; import "forge-std/Test.sol"; -import {L2TaskBase} from "src/improvements/tasks/MultisigTask.sol"; +import {SimpleBase} from "src/improvements/tasks/MultisigTask.sol"; import {ModuleManager} from "lib/safe-contracts/contracts/base/ModuleManager.sol"; -import {SuperchainAddressRegistry} from "src/improvements/SuperchainAddressRegistry.sol"; +import {SimpleAddressRegistry} from "src/improvements/SimpleAddressRegistry.sol"; import {AccountAccessParser} from "src/libraries/AccountAccessParser.sol"; /// @notice Template contract for enabling the DeputyPauseModule in a Gnosis Safe -contract EnableDeputyPauseModuleTemplate is L2TaskBase { +contract EnableDeputyPauseModuleTemplate is SimpleBase { using AccountAccessParser for *; using stdStorage for StdStorage; @@ -55,13 +55,6 @@ contract EnableDeputyPauseModuleTemplate is L2TaskBase { 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 - SuperchainAddressRegistry.ChainInfo[] memory _chains = abi.decode( - vm.parseToml(vm.readFile(taskConfigFilePath), ".l2chains"), (SuperchainAddressRegistry.ChainInfo[]) - ); - - assertEq(_chains.length, 1, "Must specify exactly one chain id to enable deputy pause module for"); } /// @notice Builds the action for enabling the module in the Safe @@ -71,18 +64,6 @@ contract EnableDeputyPauseModuleTemplate is L2TaskBase { /// @notice Validates that the module was enabled correctly. function _validate(VmSafe.AccountAccess[] memory accountAccesses, Action[] memory) internal view override { - SuperchainAddressRegistry.ChainInfo[] memory chains = superchainAddrRegistry.getChains(); - - for (uint256 i = 0; i < chains.length; i++) { - uint256 chainId = chains[i].chainId; - _validatePerChain(chainId, accountAccesses); - } - } - - /// @notice Validates that the module was enabled correctly for a given chain. - /// @param chainId The chain ID of the chain to validate - /// @param accountAccess the list of account accesses performed by this task - function _validatePerChain(uint256 chainId, VmSafe.AccountAccess[] memory accountAccess) internal view { (address[] memory modules, address nextModule) = ModuleManager(parentMultisig).getModulesPaginated(SENTINEL_MODULE, 100); @@ -104,7 +85,7 @@ contract EnableDeputyPauseModuleTemplate is L2TaskBase { ); assertEq( address(deputyGuardianModule.superchainConfig()), - superchainAddrRegistry.getAddress("SuperchainConfig", chainId), + simpleAddrRegistry.get("SuperchainConfig"), "Superchain config address not correct" ); @@ -113,11 +94,11 @@ contract EnableDeputyPauseModuleTemplate is L2TaskBase { bool moduleWriteFound; - address[] memory uniqueWrites = accountAccess.getUniqueWrites(); + address[] memory uniqueWrites = accountAccesses.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); + AccountAccessParser.StateDiff[] memory accountWrites = accountAccesses.getStateDiffFor(parentMultisig); for (uint256 i = 0; i < accountWrites.length; i++) { AccountAccessParser.StateDiff memory storageAccess = accountWrites[i]; diff --git a/test/registry/SimpleAddressRegistry.t.sol b/test/registry/SimpleAddressRegistry.t.sol index c2d5dca267..847fc2ae51 100644 --- a/test/registry/SimpleAddressRegistry.t.sol +++ b/test/registry/SimpleAddressRegistry.t.sol @@ -3,29 +3,16 @@ pragma solidity ^0.8.0; import {Test} from "forge-std/Test.sol"; import {SimpleAddressRegistry} from "src/improvements/SimpleAddressRegistry.sol"; -import {stdJson} from "forge-std/StdJson.sol"; -import {console} from "forge-std/console.sol"; -contract Helper { - // Path to test fixture files - string constant FIXTURES_PATH = "test/fixtures/SimpleAddressRegistry/"; - - // Helper function to read TOML from fixture file - function _getPath(string memory configFile) internal pure returns (string memory) { - return string.concat(FIXTURES_PATH, configFile); - } - - function _deployRegistry(string memory configFile) internal virtual returns (address) { - return address(new SimpleAddressRegistry(_getPath(configFile))); - } -} - -contract SimpleAddressRegistryTest is Helper, Test { +contract SimpleAddressRegistryTest is Test { // Test addresses address constant alice = address(1); address constant bob = address(2); address constant charlie = address(3); + // Path to test fixture files + string constant FIXTURES_PATH = "test/fixtures/SimpleAddressRegistry/"; + string registryName; // Contract name being tested. string idReturnKind; // "identifier" or "AddressInfo" @@ -37,6 +24,15 @@ contract SimpleAddressRegistryTest is Helper, Test { isSimpleAddressRegistry = true; } + // Helper function to read TOML from fixture file + function _getPath(string memory configFile) internal pure returns (string memory) { + return string.concat(FIXTURES_PATH, configFile); + } + + function _deployRegistry(string memory configFile) internal virtual returns (address) { + return address(new SimpleAddressRegistry(_getPath(configFile))); + } + function test_initialize_succeeds_withValidToml() public { SimpleAddressRegistry registry = SimpleAddressRegistry(_deployRegistry("valid_addresses.toml")); assertEq(registry.get("Alice"), alice, "10"); diff --git a/test/tasks/MultisigTask.t.sol b/test/tasks/MultisigTask.t.sol index 4a4fbdd86f..3dd5f0682e 100644 --- a/test/tasks/MultisigTask.t.sol +++ b/test/tasks/MultisigTask.t.sol @@ -5,7 +5,6 @@ import {IMulticall3} from "forge-std/interfaces/IMulticall3.sol"; import {VmSafe} from "forge-std/Vm.sol"; import {Test} from "forge-std/Test.sol"; import {stdStorage, StdStorage} from "forge-std/StdStorage.sol"; - import {IGnosisSafe, Enum} from "@base-contracts/script/universal/IGnosisSafe.sol"; import {MockTarget} from "test/tasks/mock/MockTarget.sol"; @@ -24,19 +23,19 @@ contract MultisigTaskUnitTest is Test { /// @notice variables that store the storage offset of different variables in the MultisigTask contract /// @notice storage slot for the address registry contract - bytes32 public constant ADDRESS_REGISTRY_SLOT = bytes32(uint256(34)); + bytes32 public constant ADDRESS_REGISTRY_SLOT = bytes32(uint256(35)); /// @notice storage slot for the parent multisig address - bytes32 public constant MULTISIG_SLOT = bytes32(uint256(35)); + bytes32 public constant MULTISIG_SLOT = bytes32(uint256(36)); /// @notice storage slot for the mock target contract - bytes32 public constant MOCK_TARGET_SLOT = bytes32(uint256(50)); + bytes32 public constant MOCK_TARGET_SLOT = bytes32(uint256(52)); /// @notice storage slot for the build started flag - bytes32 public constant BUILD_STARTED_SLOT = bytes32(uint256(48)); + bytes32 public constant BUILD_STARTED_SLOT = bytes32(uint256(49)); /// @notice storage slot for the target multicall address - bytes32 public constant TARGET_MULTICALL_SLOT = bytes32(uint256(49)); + bytes32 public constant TARGET_MULTICALL_SLOT = bytes32(uint256(50)); /// Test Philosophy: /// We want these tests to function as much as possible as unit tests. @@ -178,11 +177,11 @@ contract MultisigTaskUnitTest is Test { task.build(); } - function testRun() + function runTestSimulation(string memory taskConfigFilePath) public returns (VmSafe.AccountAccess[] memory accountAccesses, MultisigTask.Action[] memory actions) { - (accountAccesses, actions) = task.simulateRun(MAINNET_CONFIG); + (accountAccesses, actions) = task.simulateRun(taskConfigFilePath); (address[] memory targets, uint256[] memory values, bytes[] memory calldatas) = task.processTaskActions(actions); @@ -210,7 +209,8 @@ contract MultisigTaskUnitTest is Test { } function testSimulateFailsTxAlreadyExecuted() public { - (VmSafe.AccountAccess[] memory accountAccesses, MultisigTask.Action[] memory actions) = testRun(); + (VmSafe.AccountAccess[] memory accountAccesses, MultisigTask.Action[] memory actions) = + runTestSimulation(MAINNET_CONFIG); vm.expectRevert("MultisigTask: execute failed"); task.simulate("", actions); @@ -220,7 +220,7 @@ contract MultisigTaskUnitTest is Test { } function testGetCalldata() public { - (, MultisigTask.Action[] memory actions) = testRun(); + (, MultisigTask.Action[] memory actions) = runTestSimulation(MAINNET_CONFIG); (address[] memory targets, uint256[] memory values, bytes[] memory calldatas) = task.processTaskActions(actions); diff --git a/test/tasks/StateOverrideManager.t.sol b/test/tasks/StateOverrideManager.t.sol new file mode 100644 index 0000000000..0e100d6dca --- /dev/null +++ b/test/tasks/StateOverrideManager.t.sol @@ -0,0 +1,271 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import {Test} from "forge-std/Test.sol"; +import {LibString} from "@solady/utils/LibString.sol"; +import {IGnosisSafe} from "@base-contracts/script/universal/IGnosisSafe.sol"; + +import {MockMultisigTask} from "test/tasks/mock/MockMultisigTask.sol"; +import {Simulation} from "@base-contracts/script/universal/Simulation.sol"; +import {MultisigTask} from "src/improvements/tasks/MultisigTask.sol"; + +contract StateOverrideManagerUnitTest is Test { + function setUp() public { + vm.createSelectFork("mainnet"); + } + + string constant commonToml = "l2chains = [{name = \"OP Mainnet\", chainId = 10}]\n" "\n" + "templateName = \"DisputeGameUpgradeTemplate\"\n" "\n" + "implementations = [{gameType = 0, implementation = \"0xf691F8A6d908B58C534B624cF16495b491E633BA\", l2ChainId = 10}]\n"; + + function createTempTomlFile(string memory tomlContent) internal returns (string memory) { + string memory fileName = + string.concat(LibString.toHexString(uint256(keccak256(abi.encode(tomlContent)))), ".toml"); + vm.writeFile(fileName, tomlContent); + return fileName; + } + + function testNonceAndThresholdStateOverrideApplied() public { + // This config includes both nonce and threshold state overrides. + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = \"0x0000000000000000000000000000000000000000000000000000000000000005\", value = \"0x0000000000000000000000000000000000000000000000000000000000000FFF\"},\n", + " {key = \"0x0000000000000000000000000000000000000000000000000000000000000004\", value = \"0x0000000000000000000000000000000000000000000000000000000000000002\"}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + assertNonceIncremented(4095, task); + assertEq(IGnosisSafe(task.parentMultisig()).getThreshold(), 2, "Threshold must be 2"); + uint256 threshold = uint256(vm.load(address(task.parentMultisig()), bytes32(uint256(0x4)))); + assertEq(threshold, 2, "Threshold must be 2 using vm.load"); + vm.removeFile(fileName); + } + + function testNonceStateOverrideApplied() public { + // This config only applies a nonce override. + // 0xAAA in hex is 2730 in decimal. + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = \"0x0000000000000000000000000000000000000000000000000000000000000005\", value = \"0x0000000000000000000000000000000000000000000000000000000000000AAA\"}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + assertNonceIncremented(2730, task); + vm.removeFile(fileName); + } + + function testInvalidAddressInStateOverrideFails() public { + // Test with invalid address + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x1234 = [\n", // Invalid address + " {key = \"0x0000000000000000000000000000000000000000000000000000000000000005\", value = \"0x0000000000000000000000000000000000000000000000000000000000000001\"}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = new MockMultisigTask(); + vm.expectRevert(); + task.simulateRun(fileName); + vm.removeFile(fileName); + } + + function testDecimalKeyInConfigForStateOverridePasses() public { + // key is a decimal number (important: not surrounded by quotes) + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = 5, value = \"0x0000000000000000000000000000000000000000000000000000000000000001\"}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + assertNonceIncremented(1, task); + vm.removeFile(fileName); + } + + function testDecimalValuesInConfigForStateOverridePasses() public { + // key and value are decimal numbers (important: not surrounded by quotes) + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = 5, value = 101}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + assertNonceIncremented(101, task); + vm.removeFile(fileName); + } + + function testOnlyDefaultTenderlyStateOverridesApplied() public { + string memory fileName = createTempTomlFile(commonToml); + MultisigTask task = createAndRunTask(fileName); + + uint256 expectedNonce = task.nonce(); + assertDefaultStateOverrides(expectedNonce, 1, task); + vm.removeFile(fileName); + } + + function testUserTenderlyStateOverridesTakePrecedence() public { + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = 5, value = 100}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + + uint256 expectedNonce = 100; + Simulation.StateOverride[] memory allOverrides = assertDefaultStateOverrides(expectedNonce, 2, task); + // User defined override must be applied last + assertEq(allOverrides[1].overrides[0].key, bytes32(uint256(5)), "User defined override key must be 5"); + assertEq( + allOverrides[1].overrides[0].value, + bytes32(uint256(expectedNonce)), + "User defined override must be applied last" + ); + vm.removeFile(fileName); + } + + function testAdditionalUserStateOverridesApplied() public { + bytes32 overrideKey = bytes32(uint256(keccak256("random.slot.testAdditionalUserStateOverridesApplied")) - 1); + string memory overrideKeyString = LibString.toHexString(uint256(overrideKey)); + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = \"", + overrideKeyString, + "\", value = 9999}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + + uint256 expectedNonce = task.nonce(); + uint256 expectedTotalOverrides = 2; + Simulation.StateOverride[] memory allOverrides = + assertDefaultStateOverrides(expectedNonce, expectedTotalOverrides, task); + assertEq(allOverrides[1].overrides[0].key, overrideKey, "User override key must match expected value"); + assertEq(allOverrides[1].overrides[0].value, bytes32(uint256(9999)), "User override must be applied last"); + vm.removeFile(fileName); + } + + function testMultipleAddressStateOverridesApplied() public { + bytes32 overrideKey = bytes32(uint256(keccak256("random.slot.testAdditionalUserStateOverridesApplied")) - 1); + string memory overrideKeyString = LibString.toHexString(uint256(overrideKey)); + string memory toml = string.concat( + commonToml, + "[stateOverrides]\n", + "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A = [\n", + " {key = \"", + overrideKeyString, + "\", value = 9999}\n", + "]\n", + "0x229047fed2591dbec1eF1118d64F7aF3dB9EB290 = [\n", + " {key = \"", + overrideKeyString, + "\", value = 8888}\n", + "]" + ); + string memory fileName = createTempTomlFile(toml); + MultisigTask task = createAndRunTask(fileName); + + uint256 expectedNonce = task.nonce(); + uint256 expectedTotalOverrides = 3; // i.e. (default + 2 user defined) + Simulation.StateOverride[] memory allOverrides = + assertDefaultStateOverrides(expectedNonce, expectedTotalOverrides, task); + assertEq( + allOverrides[1].overrides[0].key, overrideKey, "First address user override key must match expected value" + ); + assertEq( + allOverrides[1].overrides[0].value, bytes32(uint256(9999)), "First address user override must be applied" + ); + assertEq( + allOverrides[2].overrides.length, + 1, + "Second address is not the parent multisig so it should only have 1 override" + ); + assertEq( + allOverrides[2].overrides[0].key, overrideKey, "Second address user override key must match expected value" + ); + assertEq( + allOverrides[2].overrides[0].value, bytes32(uint256(8888)), "Second address user override must be applied" + ); + vm.removeFile(fileName); + } + + function createAndRunTask(string memory fileName) internal returns (MultisigTask) { + MultisigTask task = new MockMultisigTask(); + task.simulateRun(fileName); + return task; + } + + function assertNonceIncremented(uint256 expectedNonce, MultisigTask task) internal view { + assertEq(task.nonce(), expectedNonce, string.concat("Expected nonce ", LibString.toString(expectedNonce))); + uint256 actualNonce = uint256(vm.load(address(task.parentMultisig()), bytes32(uint256(0x5)))); + assertEq(actualNonce, expectedNonce + 1, "Nonce must be incremented by 1 in memory after task is run"); + } + + /// @notice This function is used to assert the default state overrides for the parent multisig. + function assertDefaultStateOverrides(uint256 expectedNonce, uint256 expectedTotalOverrides, MultisigTask task) + internal + view + returns (Simulation.StateOverride[] memory allOverrides_) + { + allOverrides_ = task.getStateOverrides(address(task.parentMultisig()), task.nonce()); + + assertTrue(allOverrides_.length >= 1, "Must be at least 1 override (default + user defined)"); + assertEq( + allOverrides_.length, + expectedTotalOverrides, + string.concat("Total number of overrides must be ", LibString.toString(expectedTotalOverrides)) + ); + Simulation.StateOverride memory defaultOverride = allOverrides_[0]; + assertEq( + defaultOverride.contractAddress, + address(task.parentMultisig()), + "Contract address must be the parent multisig for the default override" + ); + assertTrue(defaultOverride.overrides.length == 5, "Default override must have 5 overrides"); + assertEq(defaultOverride.overrides[0].key, bytes32(uint256(0x4)), "Must contain a threshold override"); + assertEq(defaultOverride.overrides[0].value, bytes32(uint256(0x1)), "Threshold override must be 1"); + assertEq(defaultOverride.overrides[1].key, bytes32(uint256(0x5)), "Must contain a nonce override"); + assertEq(defaultOverride.overrides[1].value, bytes32(expectedNonce), "Nonce override must match expected value"); + assertEq(defaultOverride.overrides[2].key, bytes32(uint256(0x3)), "Must contain an owner count override"); + assertEq(defaultOverride.overrides[2].value, bytes32(uint256(0x1)), "Owner count override must be 1"); + // Verify owner mapping overrides + assertEq( + defaultOverride.overrides[3].key, + bytes32(uint256(0xe90b7bceb6e7df5418fb78d8ee546e97c83a08bbccc01a0644d599ccd2a7c2e0)), + "Must contain first owner mapping override" + ); + assertEq( + defaultOverride.overrides[3].value, + bytes32(uint256(0x0000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e1496)), + "Incorrect first owner mapping override" + ); + assertEq( + defaultOverride.overrides[4].key, + bytes32(uint256(0x6e10ff27cae71a13525bd61167857e5c982b4674c8e654900e4e9d5035811f78)), + "Must contain second owner mapping override" + ); + assertEq( + defaultOverride.overrides[4].value, + bytes32(uint256(0x1)), + "Must contain second owner mapping override value" + ); + return allOverrides_; + } +} diff --git a/test/tasks/mock/MockTarget.sol b/test/tasks/mock/MockTarget.sol index a7a37ba31f..62f18026e6 100644 --- a/test/tasks/mock/MockTarget.sol +++ b/test/tasks/mock/MockTarget.sol @@ -5,7 +5,7 @@ import {Test} from "forge-std/Test.sol"; contract MockTarget is Test { address public task; - bytes32 public START_SNAPSHOT_SLOT = bytes32(uint256(45)); + bytes32 public START_SNAPSHOT_SLOT = bytes32(uint256(46)); function setTask(address _task) public { task = _task; diff --git a/test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml b/test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml index d346e93830..148e72457c 100644 --- a/test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml +++ b/test/tasks/mock/configs/EnableDeputyPauseModuleTemplate.toml @@ -1,7 +1,4 @@ # this is the file used to determine the network configuration - -l2chains = [{name = "OP Testnet", chainId = 11155420}] - templateName = "EnableDeputyPauseModuleTemplate" newModule = "0x62f3972c56733aB078F0764d2414DfCaa99d574c"