-
Notifications
You must be signed in to change notification settings - Fork 128
Safer safes migrate to LivenessModule2 template #1298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
c414559
WIP on jb/safer-safes
JosepBove 4657493
fix: typos
JosepBove 1dde5ca
feat: regression tests and finish template
JosepBove 090e061
fmt
JosepBove 16dafa0
Merge remote-tracking branch 'origin/main' into jb/safer-safes
JosepBove 3ee2dfc
Merge branch 'main' into jb/safer-safes
JosepBove 1675a6c
Merge branch 'main' into jb/safer-safes
JosepBove 52ddc73
Merge branch 'main' into jb/safer-safes
JosepBove 50e031e
fix: block
JosepBove 445b4ef
Merge remote-tracking branch 'origin/jb/safer-safes' into jb/safer-safes
JosepBove File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
|
JosepBove marked this conversation as resolved.
|
||
|
|
||
| // 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]; | ||
| } | ||
| } | ||
|
JosepBove marked this conversation as resolved.
|
||
|
|
||
| revert("Module not found in list"); | ||
|
JosepBove marked this conversation as resolved.
|
||
| } | ||
|
|
||
| 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"); | ||
|
JosepBove marked this conversation as resolved.
|
||
|
|
||
| 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"); | ||
|
JosepBove marked this conversation as resolved.
|
||
| } | ||
|
|
||
| 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" | ||
| ); | ||
|
|
||
|
alcueca marked this conversation as resolved.
|
||
| bytes32 guardSlot = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8; | ||
| address guardAddress; | ||
| bytes32 value = vm.load(multisig, guardSlot); | ||
| assembly { | ||
| guardAddress := value | ||
| } | ||
| require(guardAddress == address(0), "Validation failed: Guard was not removed"); | ||
|
JosepBove marked this conversation as resolved.
|
||
|
|
||
| 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"); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| TENDERLY_GAS=16700000 | ||
| FORK_BLOCK_NUMBER=9691830 | ||
| NESTED_SAFE_NAME_DEPTH_1=foundation |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
16 changes: 16 additions & 0 deletions
16
test/tasks/example/sep/030-migrate-to-liveness2/config.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| templateName = "MigrateToLiveness2" | ||
|
|
||
| [addresses] | ||
| saferSafes="0xdffFe9Dc096938fb156A54e67D09f73f5514Eb16" | ||
|
alcueca marked this conversation as resolved.
|
||
| targetSafe="0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E" | ||
| currentLivenessModule="0xB78eA2a288fca20D7f499F8a9eE48ff506609855" | ||
|
|
||
|
|
||
| [livenessModule] | ||
| livenessResponsePeriod="1" | ||
|
alcueca marked this conversation as resolved.
|
||
| fallbackOwner="0xe934Dc97E347C6aCef74364B50125bb8689c40ff" | ||
|
|
||
| [stateOverrides] | ||
| 0xB2DEfc35a51E4f2126667A9FC8D941202077aC0E = [ # targetSafe - temporarily remove guard for simulation | ||
| {key = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8", value = 0} # guard slot | ||
| ] | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.