diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7d18ff44b..5cbb02060 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,5 +1,5 @@ * @ethereum-optimism/evm-safety -/runbooks @ethereum-optimism/devrel @ethereum-optimism/evm-safety - +/runbooks @ethereum-optimism/solutions @ethereum-optimism/evm-safety /src @ethereum-optimism/contract-reviewers @ethereum-optimism/evm-safety +/src/tasks/sep @ethereum-optimism/solutions diff --git a/src/template/MigrateToLiveness2.sol b/src/template/MigrateToLiveness2.sol new file mode 100644 index 000000000..fbb1468c1 --- /dev/null +++ b/src/template/MigrateToLiveness2.sol @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.15; + +import {SimpleTaskBase} from "src/tasks/types/SimpleTaskBase.sol"; +import {VmSafe} from "forge-std/Vm.sol"; +import {stdToml} from "forge-std/StdToml.sol"; +import {LibString} from "@solady/utils/LibString.sol"; +import {Action, TemplateConfig, TaskType, TaskPayload, SafeData} from "src/libraries/MultisigTypes.sol"; + +interface ISaferSafes { + struct ModuleConfig { + uint256 livenessResponsePeriod; + address fallbackOwner; + } + + function enableModule(address _module) external; + function setGuard(address _guard) external; + function configureTimelockGuard(uint256 _timelockDelay) external; + function configureLivenessModule(ModuleConfig memory _moduleConfig) external; + function version() external view returns (string memory); + function getModulesPaginated(address start, uint256 pageSize) + external + view + returns (address[] memory array, address next); + function disableModule(address prevModule, address module) external; + function isModuleEnabled(address module) external view returns (bool); + function getGuard() external view returns (address); + function livenessSafeConfiguration(address safe) external view returns (ModuleConfig memory); +} + +interface IMultisig { + function version() external view returns (string memory); +} + +contract MigrateToLiveness2 is SimpleTaskBase { + using stdToml for string; + using LibString for string; + + address public saferSafes; + address public multisig; + address public currentLivenessModule; + + uint256 public timelockDelay; + uint256 public livenessResponsePeriod; + address public fallbackOwner; + + function _taskStorageWrites() internal pure override returns (string[] memory) { + string[] memory writes = new string[](2); + writes[0] = "targetSafe"; // Safe being modified (enableModule, etc.) + writes[1] = "saferSafes"; // SaferSafes contract (configureLivenessModule) + return writes; + } + + function _getCodeExceptions() internal view override returns (address[] memory) {} + + function safeAddressString() public pure override returns (string memory) { + return "targetSafe"; // References the custom safe from config.toml + } + + /// @notice Find the previous module in the linked list + /// @param moduleToFind The module to find the previous module for + /// @return The address of the previous module in the linked list + function _findPrevModule(address moduleToFind) internal view returns (address) { + address SENTINEL_MODULES = address(0x1); + + (address[] memory modules,) = ISaferSafes(multisig).getModulesPaginated(SENTINEL_MODULES, 100); + + // If the module is the first in the list, previous is sentinel + if (modules.length > 0 && modules[0] == moduleToFind) { + return SENTINEL_MODULES; + } + + // Otherwise, find the module and return the previous one + for (uint256 i = 1; i < modules.length; i++) { + if (modules[i] == moduleToFind) { + return modules[i - 1]; + } + } + + revert("Module not found in list"); + } + + function _templateSetup(string memory taskConfigFilePath, address rootSafe) internal override { + super._templateSetup(taskConfigFilePath, rootSafe); + string memory tomlContent = vm.readFile(taskConfigFilePath); + + saferSafes = tomlContent.readAddress(".addresses.saferSafes"); + multisig = tomlContent.readAddress(".addresses.targetSafe"); + currentLivenessModule = tomlContent.readAddress(".addresses.currentLivenessModule"); + + livenessResponsePeriod = tomlContent.readUint(".livenessModule.livenessResponsePeriod"); + fallbackOwner = tomlContent.readAddress(".livenessModule.fallbackOwner"); + + require(address(saferSafes).code.length > 0, "SaferSafes does not have code"); + require(address(currentLivenessModule).code.length > 0, "Current LivenessModule does not have code"); + require(livenessResponsePeriod > 0, "Liveness response period must be greater than 0"); + } + + function _build(address) internal override { + // Remove the guard first so it doesn't interfere with subsequent operations + ISaferSafes(multisig).setGuard(address(0)); + + // Enable SaferSafes as a module on the safe + ISaferSafes(multisig).enableModule(saferSafes); + + // Configure the liveness module on SaferSafes + ISaferSafes.ModuleConfig memory moduleConfig = + ISaferSafes.ModuleConfig({livenessResponsePeriod: livenessResponsePeriod, fallbackOwner: fallbackOwner}); + ISaferSafes(saferSafes).configureLivenessModule(moduleConfig); + + // Remove the old liveness module + address prevModule = _findPrevModule(currentLivenessModule); + ISaferSafes(multisig).disableModule(prevModule, currentLivenessModule); + } + + function _validate(VmSafe.AccountAccess[] memory, Action[] memory, address) internal view override { + require( + ISaferSafes(multisig).isModuleEnabled(saferSafes), "Validation failed: SaferSafes module is not enabled" + ); + + require( + !ISaferSafes(multisig).isModuleEnabled(currentLivenessModule), + "Validation failed: Old liveness module is still enabled" + ); + + bytes32 guardSlot = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8; + address guardAddress; + bytes32 value = vm.load(multisig, guardSlot); + assembly { + guardAddress := value + } + require(guardAddress == address(0), "Validation failed: Guard was not removed"); + + ISaferSafes.ModuleConfig memory config = ISaferSafes(saferSafes).livenessSafeConfiguration(multisig); + + require( + config.livenessResponsePeriod == livenessResponsePeriod, + "Validation failed: Liveness response period mismatch" + ); + + require(config.fallbackOwner == fallbackOwner, "Validation failed: Fallback owner mismatch"); + } +} diff --git a/test/tasks/Regression.t.sol b/test/tasks/Regression.t.sol index ef26366ea..ce08bbb96 100644 --- a/test/tasks/Regression.t.sol +++ b/test/tasks/Regression.t.sol @@ -41,6 +41,7 @@ import {OPCMUpgradeV410} from "src/template/OPCMUpgradeV410.sol"; import {OPCMUpgradeSuperchainConfigV410} from "src/template/OPCMUpgradeSuperchainConfigV410.sol"; import {L1PortalExecuteL2Call} from "src/template/L1PortalExecuteL2Call.sol"; import {AddGameTypeTemplate} from "src/template/AddGameTypeTemplate.sol"; +import {MigrateToLiveness2} from "src/template/MigrateToLiveness2.sol"; import {RevShareUpgradeAndSetup} from "src/template/RevShareUpgradeAndSetup.sol"; import {RevShareSetup} from "src/template/RevShareSetup.sol"; @@ -1070,10 +1071,10 @@ contract RegressionTest is Test { } /// @notice Expected call data and data to sign generated by manually running the RevShareUpgradeAndSetup template at block 9628956 on Sepolia. - /// Simulate from task directory (test/tasks/example/sep/030-revshare-upgrade-and-setup) with: + /// Simulate from task directory (test/tasks/example/sep/032-revshare-upgrade-and-setup) with: /// SIMULATE_WITHOUT_LEDGER=1 just --dotenv-path $(pwd)/.env --justfile ../../../../../src/improvements/single.just simulate function testRegressionCallDataMatches_RevShareUpgradeAndSetupSepolia() public { - string memory taskConfigFilePath = "test/tasks/example/sep/030-revshare-upgrade-and-setup/config.toml"; + string memory taskConfigFilePath = "test/tasks/example/sep/032-revshare-upgrade-and-setup/config.toml"; string memory expectedCallData = "0x82ad56cb000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000890c61c7f3f40b851ebcaacfa879c6075426419d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000e41a80dc380000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000100000000000000000000000016fc5058f25648194471939df75cf27a2fdc48bc0000000000000000000000000000000000000000000000000000000000055730000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000c3500000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000"; MultisigTask multisigTask = new RevShareUpgradeAndSetup(); @@ -1176,4 +1177,37 @@ contract RegressionTest is Test { assertEq(keccak256(bytes(dataToSign)), keccak256(bytes(expectedDataToSign[i]))); } } + + /// @notice Expected call data and data to sign generated by manually running the MigrateToLiveness2 at block 9691830 on sepolia. + /// Simulate from task directory (test/tasks/example/sep/030-migrate-to-liveness2/) with: + /// SIMULATE_WITHOUT_LEDGER=true just --justfile ../../../../../src/justfile simulate + function testRegressionCallDataMatches_MigrateToLiveness2() public { + address rootSafe = address(0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E); + string memory taskConfigFilePath = "test/tasks/example/sep/030-migrate-to-liveness2/config.toml"; + // Calldata generated by manually running the MigrateToLiveness2 template at block 9691830 on sepolia. + string memory expectedCallData = + "0x174dea71000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000002400000000000000000000000000000000000000000000000000000000000000340000000000000000000000000b2defc35a51e4f2126667a9fc8d941202077ac0e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000024e19a9dd9000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b2defc35a51e4f2126667a9fc8d941202077ac0e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000024610b5925000000000000000000000000dfffe9dc096938fb156a54e67d09f73f5514eb1600000000000000000000000000000000000000000000000000000000000000000000000000000000dfffe9dc096938fb156a54e67d09f73f5514eb16000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000004405ccf6060000000000000000000000000000000000000000000000000000000000000001000000000000000000000000e934dc97e347c6acef74364b50125bb8689c40ff00000000000000000000000000000000000000000000000000000000000000000000000000000000b2defc35a51e4f2126667a9fc8d941202077ac0e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000044e009cfde000000000000000000000000dfffe9dc096938fb156a54e67d09f73f5514eb16000000000000000000000000b78ea2a288fca20d7f499f8a9ee48ff50660985500000000000000000000000000000000000000000000000000000000"; + + MultisigTask multisigTask = new MigrateToLiveness2(); + + address[] memory allSafes = MultisigTaskTestHelper.getAllSafes(rootSafe); + (Action[] memory actions, uint256[] memory allOriginalNonces) = + _setupAndSimulate(taskConfigFilePath, 9691830, "sepolia", multisigTask, allSafes); + + bytes memory rootSafeCalldata = + _assertCallDataMatches(multisigTask, actions, allSafes, allOriginalNonces, expectedCallData); + uint256 rootSafeNonce = allOriginalNonces[allOriginalNonces.length - 1]; + + // Data to sign generated by manually running the MigrateToLiveness2 template at block 9691830 on sepolia. + string memory expectedDataToSign = + "0x1901bce54de085c4d47267dbd131a558f3e07f35d496718f397e1158eb30a6b28ae02569065845998f134d339270fcc5b81ac10c31ff00a9e4aba0a3a1f98461bbe1"; + string memory dataToSign = vm.toString( + GnosisSafeHashes.getEncodedTransactionData(rootSafe, rootSafeCalldata, 0, rootSafeNonce, MULTICALL3_ADDRESS) + ); + // assert that the data to sign generated in simulate is the same as the expected data to sign + assertEq(keccak256(bytes(dataToSign)), keccak256(bytes(expectedDataToSign))); + _assertDataToSignSingleMultisig( + rootSafe, rootSafeCalldata, expectedDataToSign, rootSafeNonce, MULTICALL3_ADDRESS + ); + } } diff --git a/test/tasks/example/sep/030-migrate-to-liveness2/.env b/test/tasks/example/sep/030-migrate-to-liveness2/.env new file mode 100644 index 000000000..de9ae20c7 --- /dev/null +++ b/test/tasks/example/sep/030-migrate-to-liveness2/.env @@ -0,0 +1,3 @@ +TENDERLY_GAS=16700000 +FORK_BLOCK_NUMBER=9691830 +NESTED_SAFE_NAME_DEPTH_1=foundation diff --git a/test/tasks/example/sep/030-migrate-to-liveness2/README.md b/test/tasks/example/sep/030-migrate-to-liveness2/README.md new file mode 100644 index 000000000..d6f820011 --- /dev/null +++ b/test/tasks/example/sep/030-migrate-to-liveness2/README.md @@ -0,0 +1,48 @@ +# MigrateToLiveness2 Task + +## Overview + +This task migrates a Safe from the v1 liveness module (which combined guard and module functionality) to the v2 SaferSafes-based liveness system. + +## What This Task Does + +1. Removes the guard (sets it to `address(0)`) +2. Enables the SaferSafes module +3. Configures the liveness module settings +4. Disables the old liveness module + +## State Override Explanation + +### Why do we need `stateOverrides` in the config? + +The config includes a state override that temporarily removes the guard during simulation: + +```toml +[stateOverrides] +0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E = [ + {key = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8", value = 0} +] +``` + +**This is only needed for simulation, NOT for production.** + +### The Problem + +The old liveness guard has a `checkTransaction()` hook that runs **before** every Safe transaction. This guard checks that `msg.sender` (the caller of `execTransaction`) is a Safe owner. + +**In simulation:** +- The MultisigTask framework contract calls `execTransaction()` +- The guard sees `msg.sender = MultisigTask contract` (not an owner) +- The guard blocks the transaction with error `TimelockGuard_NotOwner()` + +**In production (using `just sign` and `just execute`):** +- Owners sign the transaction using `just sign` +- An owner executes the transaction using `just execute` +- The guard sees `msg.sender = owner` +- The guard allows the transaction to proceed + +### The Solution + +The state override temporarily sets the guard slot to `address(0)` during simulation, bypassing the guard check. This allows the simulation framework to test the transaction logic without needing to be an owner. + +**Important:** This is purely a simulation workaround. In production, an owner must run `just execute` so the guard check passes. The transaction will succeed without any override. diff --git a/test/tasks/example/sep/030-migrate-to-liveness2/config.toml b/test/tasks/example/sep/030-migrate-to-liveness2/config.toml new file mode 100644 index 000000000..a31ebb214 --- /dev/null +++ b/test/tasks/example/sep/030-migrate-to-liveness2/config.toml @@ -0,0 +1,16 @@ +templateName = "MigrateToLiveness2" + +[addresses] +saferSafes="0xdffFe9Dc096938fb156A54e67D09f73f5514Eb16" +targetSafe="0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E" +currentLivenessModule="0xB78eA2a288fca20D7f499F8a9eE48ff506609855" + + +[livenessModule] +livenessResponsePeriod="1" +fallbackOwner="0xe934Dc97E347C6aCef74364B50125bb8689c40ff" + +[stateOverrides] +0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E = [ # targetSafe - temporarily remove guard for simulation + {key = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8", value = 0} # guard slot +] \ No newline at end of file diff --git a/test/tasks/example/sep/030-revshare-upgrade-and-setup/.env b/test/tasks/example/sep/032-revshare-upgrade-and-setup/.env similarity index 100% rename from test/tasks/example/sep/030-revshare-upgrade-and-setup/.env rename to test/tasks/example/sep/032-revshare-upgrade-and-setup/.env diff --git a/test/tasks/example/sep/030-revshare-upgrade-and-setup/config.toml b/test/tasks/example/sep/032-revshare-upgrade-and-setup/config.toml similarity index 100% rename from test/tasks/example/sep/030-revshare-upgrade-and-setup/config.toml rename to test/tasks/example/sep/032-revshare-upgrade-and-setup/config.toml