Skip to content
Merged
Show file tree
Hide file tree
Changes from 71 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
82e8ce3
Add SaferSafes as child of the module and guard
maurelian Oct 6, 2025
b6a5561
Add ISaferSafes
maurelian Oct 9, 2025
3a33850
Test comment and assertion fixes
maurelian Oct 9, 2025
81399b2
Improve comments
maurelian Oct 9, 2025
05aa13d
Make LivenessModule2 and TimelockGuard abstract
maurelian Oct 9, 2025
d35175c
fix test contract name
maurelian Oct 9, 2025
3da8e5b
Move semver to SaferSafes
maurelian Oct 9, 2025
64e0153
Disable the guard and module upon ownership transfer
maurelian Oct 9, 2025
300ab95
Add _disableThisGuard function
maurelian Oct 9, 2025
5efbf2a
Update tests
maurelian Oct 9, 2025
6e2acad
Add config resets
maurelian Oct 9, 2025
8f161cf
fmt
maurelian Oct 9, 2025
b5461ff
fix test_changeOwnershipToFallback_canRechallenge_succeeds
maurelian Oct 9, 2025
46fb155
Simplify by clearing config directly
maurelian Oct 9, 2025
993232a
Put _disableThisGuard into child contract
maurelian Oct 9, 2025
7f1d9ef
Add timelockDelay reset on _disableThisGuard
maurelian Oct 9, 2025
aa50113
semver-lock
maurelian Oct 9, 2025
6cb5167
Move _disableThisGuard logic into TimelockGuard
maurelian Oct 9, 2025
74113a7
clear livenessSafeConfig at tend of _disableThisModule
maurelian Oct 9, 2025
7242c80
Clarify use of SENTINEL_OWNER
maurelian Oct 9, 2025
cfb6110
Fix the ordering of the disableGuard and disableModule calls
maurelian Oct 10, 2025
3b1132b
semver-lock
maurelian Oct 10, 2025
48663a6
remove unused imports
maurelian Oct 10, 2025
c6ea913
rename _disableThisGuard to _disableGuard
maurelian Oct 10, 2025
819a246
bump semver
maurelian Oct 10, 2025
fcd9fa4
Add test to remove unrelated guard
maurelian Oct 14, 2025
a643404
Add SENTINEL_MODULE constant
maurelian Oct 14, 2025
a2d6019
Clean up using ternary if
maurelian Oct 14, 2025
539dd58
Reset cancellationThreshold to 0 on changeOwnership
maurelian Oct 14, 2025
9db0218
Fix moduleFound if/else handling
maurelian Oct 14, 2025
f7763dc
Clear pending transactions
maurelian Oct 15, 2025
f0855e5
Pre-pr fixes
maurelian Oct 15, 2025
6b2b25d
Add test contract to test name lint exclusions
maurelian Oct 15, 2025
cb58df1
fix name of test contract
maurelian Oct 15, 2025
8dd8e55
Move _disableGuard impl into TimelockGuard
maurelian Oct 15, 2025
a36b79e
Add missing natspec
maurelian Oct 15, 2025
62fed33
Add gas limit testing on changeOwnershipToFallback
maurelian Oct 15, 2025
bd03288
Remove interfaces for abstract contracts
maurelian Oct 15, 2025
97e49d2
Merge branch 'develop' into jm/disable-extensions-on-transfer
maurelian Oct 15, 2025
f75e019
Move state changes out into internal _clearLivenessModule
maurelian Oct 15, 2025
b50c952
Improve names on the internal _disableX methods
maurelian Oct 15, 2025
866067b
Add clearTimelockGuard function
maurelian Oct 16, 2025
1d4fdda
Add _disableGuard helper to TLG tests
maurelian Oct 16, 2025
220a421
Limit number of transactions cancelled to 100
maurelian Oct 16, 2025
275f950
Revert "Remove interfaces for abstract contracts"
maurelian Oct 16, 2025
e4b4457
Move livenessModule2 address into TestUtils
maurelian Oct 16, 2025
4bf722e
Reduce diff somewhat
maurelian Oct 16, 2025
67f13d2
Remove unused arg
maurelian Oct 16, 2025
f62065a
Update packages/contracts-bedrock/src/safe/TimelockGuard.sol
maurelian Oct 16, 2025
415d98f
Fix iface
maurelian Oct 16, 2025
9b0e831
update abi for iface fix
maurelian Oct 16, 2025
4ee99aa
Do not clear or disable the module during ownership transfer
maurelian Oct 16, 2025
75430d4
Fix inaccurate comment on _disableAndClearGuard
maurelian Oct 16, 2025
b459c34
Further improve comment
maurelian Oct 16, 2025
da19fe8
remove unused import
maurelian Oct 16, 2025
491f484
fix test name
maurelian Oct 16, 2025
5d36c21
Merge branch 'develop' into jm/disable-extensions-on-transfer
maurelian Oct 16, 2025
e6a057b
Merge branch 'develop' into jm/disable-extensions-on-transfer
maurelian Oct 16, 2025
56b439d
Do not clear guard during changeOwnershipToFallback
maurelian Oct 16, 2025
4f499e8
Remove unused SENTINEL_MODULE var
maurelian Oct 16, 2025
d266d12
Remove dangling comment
maurelian Oct 16, 2025
9f50cdc
Revert "Remove dangling comment"
maurelian Oct 16, 2025
3ce855f
Fix whitespace
maurelian Oct 16, 2025
7a93d4e
Merge branch 'develop' into jm/disable-extensions-on-transfer
maurelian Oct 16, 2025
af77e1b
remove unnecessary internal _clearTimelockGuard function
maurelian Oct 16, 2025
39434fc
Address feedback
maurelian Oct 17, 2025
3f2595b
Add missing assertion
maurelian Oct 17, 2025
6977109
Move guard slot into constants
maurelian Oct 17, 2025
0b38113
semver-lock
maurelian Oct 17, 2025
e525173
Remove LivenessModule from semver-lock
maurelian Oct 17, 2025
07ecba6
fix: fmt, semver-lock, unused imports
maurelian Oct 17, 2025
3f2a72c
Remove unused variable
maurelian Oct 17, 2025
7369668
fix semver lock by resetting old LivenessModule
maurelian Oct 17, 2025
0434bfa
fix unused import
maurelian Oct 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ interface ITimelockGuard {
event TransactionScheduled(address indexed safe, bytes32 indexed txHash, uint256 executionTime);
event TransactionExecuted(address indexed safe, bytes32 txHash);
event Message(string message);
event TransactionsNotCancelled(address indexed safe, uint256 uncancelledCount);

function cancelTransaction(address _safe, bytes32 _txHash, uint256 _nonce, bytes memory _signatures) external;
function signCancellation(bytes32 _txHash) external;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,5 @@ contracts = [
"OptimismPortal2_UpgradeInterop_Test", # Interop tests hosted in the OptimismPortal2 test file
"TransactionBuilder", # Transaction builder helper library in TimelockGuard test file
"Constants_Test", # Invalid naming pattern - doesn't specify function or Uncategorized
"LivenessModule2_TestUtils", # Test utils library in LivenessModule2 test file
]
31 changes: 31 additions & 0 deletions packages/contracts-bedrock/snapshots/abi/SaferSafes.json
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [],
"name": "clearTimelockGuard",
"outputs": [],
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
Expand Down Expand Up @@ -786,6 +793,25 @@
"name": "TransactionScheduled",
"type": "event"
},
{
"anonymous": false,
"inputs": [
{
"indexed": true,
"internalType": "contract GnosisSafe",
"name": "safe",
"type": "address"
},
{
"indexed": false,
"internalType": "uint256",
"name": "uncancelledCount",
"type": "uint256"
}
],
"name": "TransactionsNotCancelled",
"type": "event"
},
{
"inputs": [],
"name": "LivenessModule2_ChallengeAlreadyExists",
Expand Down Expand Up @@ -861,6 +887,11 @@
"name": "TimelockGuard_GuardNotEnabled",
"type": "error"
},
{
"inputs": [],
"name": "TimelockGuard_GuardStillEnabled",
"type": "error"
},
{
"inputs": [],
"name": "TimelockGuard_InvalidTimelockDelay",
Expand Down
6 changes: 3 additions & 3 deletions packages/contracts-bedrock/snapshots/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@
},
"src/safe/LivenessModule.sol:LivenessModule": {
"initCodeHash": "0xa4a06e8778dbb6883ece8f56538ba15bc01b3031bba9a12ad9d187e7c8aaa942",
"sourceCodeHash": "0x950725f8b9ad9bb3b6b5e836f67e18db824a7864bac547ee0eeba88ada3de0e9"
"sourceCodeHash": "0x5bcd9f0f06a4d23183401702426c2907b1882813fd56f420c1f7f77f3bf771b2"
},
"src/safe/SaferSafes.sol:SaferSafes": {
"initCodeHash": "0x22730f6ebe10e0d78b23b3e0f3d58f867f420039981455b09a508755e512c127",
"sourceCodeHash": "0xe3d2fd50724baf6d5e83e2c9d89fcfcbc678d966187bd38e2d9a840716bcafa3"
"initCodeHash": "0x68567829042bbd450ffd477848ddcda288e36714846fdcc02c315f2d0d332da6",
"sourceCodeHash": "0x06fc6d5df3c5769b645ff319d8b94415e501711c19cb0b1bce2631771d22294f"
},
"src/universal/OptimismMintableERC20.sol:OptimismMintableERC20": {
"initCodeHash": "0x3c85eed0d017dca8eda6396aa842ddc12492587b061e8c756a8d32c4610a9658",
Expand Down
4 changes: 4 additions & 0 deletions packages/contracts-bedrock/src/libraries/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ library Constants {
/// @dev `bytes32(uint256(keccak256('eip1967.proxy.admin')) - 1)`
bytes32 internal constant PROXY_OWNER_ADDRESS = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;

/// @notice The storage slot that holds the guard address in Safe contracts.
/// @dev `keccak256("guard_manager.guard.address")`
bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;

/// @notice The address that represents ether when dealing with ERC20 token addresses.
address internal constant ETHER = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

Expand Down
10 changes: 5 additions & 5 deletions packages/contracts-bedrock/src/safe/LivenessModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import { LivenessGuard } from "src/safe/LivenessGuard.sol";
// Interfaces
import { ISemver } from "interfaces/universal/ISemver.sol";

// Libraries
import { Constants } from "src/libraries/Constants.sol";

/// @title LivenessModule
/// @notice This module is intended to be used in conjunction with the LivenessGuard. In the event
/// that an owner of the safe is not recorded by the guard during the liveness interval,
Expand Down Expand Up @@ -53,10 +56,6 @@ contract LivenessModule is ISemver {
/// This can be updated by replacing with a new module.
address internal immutable FALLBACK_OWNER;

/// @notice The storage slot used in the safe to store the guard address
/// keccak256("guard_manager.guard.address")
uint256 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;

/// @notice Semantic version.
/// @custom:semver 1.2.2
string public constant version = "1.2.2";
Expand Down Expand Up @@ -260,7 +259,8 @@ contract LivenessModule is ISemver {

// Check that the guard has not been changed
require(
address(LIVENESS_GUARD) == address(uint160(uint256(bytes32(SAFE.getStorageAt(GUARD_STORAGE_SLOT, 1))))),
address(LIVENESS_GUARD)
== address(uint160(uint256(bytes32(SAFE.getStorageAt(uint256(Constants.GUARD_STORAGE_SLOT), 1))))),
"LivenessModule: guard has been changed"
);
}
Expand Down
15 changes: 14 additions & 1 deletion packages/contracts-bedrock/src/safe/LivenessModule2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity 0.8.15;
import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol";
import { Enum } from "safe-contracts/common/Enum.sol";
import { OwnerManager } from "safe-contracts/base/OwnerManager.sol";
import { GuardManager } from "safe-contracts/base/GuardManager.sol";

/// @title LivenessModule2
/// @notice This module allows challenge-based ownership transfer to a fallback owner
Expand Down Expand Up @@ -146,6 +147,7 @@ abstract contract LivenessModule2 {

/// @notice Internal helper function which can be overriden in a child contract to check if the guard's
/// configuration is valid in the context of other extensions that are enabled on the Safe.
/// @param _safe The Safe instance to check the configuration against
function _checkCombinedConfig(Safe _safe) internal view virtual;
Comment thread
maurelian marked this conversation as resolved.

/// @notice Clears the module configuration for a Safe.
Expand Down Expand Up @@ -282,8 +284,19 @@ abstract contract LivenessModule2 {

// Reset the challenge state to allow a new challenge
delete challengeStartTime[_safe];

emit ChallengeSucceeded(_safe, livenessSafeConfiguration[_safe].fallbackOwner);

// Disable the guard
// Note that this will remove whichever guard is currently set on the Safe,
Comment thread
maurelian marked this conversation as resolved.
// even if it is not the SaferSafes guard. This is intentional, as it is possible that the guard
// itself was the cause of the liveness failure which resulted in the transfer of ownership to
// the fallback owner.
targetSafe.execTransactionFromModule({
to: _safe,
value: 0,
operation: Enum.Operation.Call,
data: abi.encodeCall(GuardManager.setGuard, (address(0)))
});
}

/// @notice Asserts that the module is configured for the given Safe.
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/src/safe/SaferSafes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { ISemver } from "interfaces/universal/ISemver.sol";
/// functionality is not desired, then there is no need to enable or configure it.
contract SaferSafes is LivenessModule2, TimelockGuard, ISemver {
/// @notice Semantic version.
/// @custom:semver 1.0.0
string public constant version = "1.0.0";
/// @custom:semver 1.1.0
string public constant version = "1.1.0";
Comment thread
maurelian marked this conversation as resolved.
Comment thread
maurelian marked this conversation as resolved.

/// @notice Error for when the liveness response period is insufficient.
error SaferSafes_InsufficientLivenessResponsePeriod();
Expand Down
63 changes: 60 additions & 3 deletions packages/contracts-bedrock/src/safe/TimelockGuard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Guard as IGuard } from "safe-contracts/base/GuardManager.sol";
// Libraries
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { SemverComp } from "src/libraries/SemverComp.sol";
import { Constants } from "src/libraries/Constants.sol";

/// @title TimelockGuard
/// @notice This guard provides timelock functionality for Safe transactions
Expand Down Expand Up @@ -149,6 +150,14 @@ abstract contract TimelockGuard is IGuard {
/// @notice Error for when the contract is not at least version 1.3.0
error TimelockGuard_InvalidVersion();

/// @notice Error for when trying to clear guard while it is still enabled
error TimelockGuard_GuardStillEnabled();

/// @notice Emitted when some transactions are not cancelled.
/// @param safe The Safe whose transactions are not cancelled.
/// @param uncancelledCount The number of transactions that are not cancelled.
event TransactionsNotCancelled(Safe indexed safe, uint256 uncancelledCount);

/// @notice Emitted when a Safe configures the guard
/// @param safe The Safe whose guard is configured.
/// @param timelockDelay The timelock delay in seconds.
Expand Down Expand Up @@ -196,9 +205,7 @@ abstract contract TimelockGuard is IGuard {
/// @param _safe The Safe address
/// @return The current guard address
function _isGuardEnabled(Safe _safe) internal view returns (bool) {
// keccak256("guard_manager.guard.address") from GuardManager
bytes32 guardSlot = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
address guard = abi.decode(_safe.getStorageAt(uint256(guardSlot), 1), (address));
address guard = abi.decode(_safe.getStorageAt(uint256(Constants.GUARD_STORAGE_SLOT), 1), (address));
return guard == address(this);
}

Expand Down Expand Up @@ -453,6 +460,56 @@ abstract contract TimelockGuard is IGuard {
_checkCombinedConfig(callingSafe);
}

/// @notice Clears the timelock guard configuration for a Safe.
/// @dev Note: Clearing the configuration also cancels all pending transactions.
/// This function is intended for use when a Safe wants to permanently remove
/// the TimelockGuard configuration. Typical usage pattern:
/// 1. Safe disables the guard via GuardManager.setGuard(address(0)).
/// 2. Safe calls this clearTimelockGuard() function to remove stored configuration.
/// 3. If Safe later re-enables the guard, it must call configureTimelockGuard() again.
function clearTimelockGuard() external {
Comment thread
maurelian marked this conversation as resolved.
Safe callingSafe = Safe(payable(msg.sender));
Comment thread
maurelian marked this conversation as resolved.
SafeState storage safeState = _safeState[callingSafe];

// Check if the calling safe has configuration set
if (safeState.timelockDelay == 0) {
revert TimelockGuard_GuardNotConfigured();
}

// Check that this guard is NOT enabled on the calling Safe
// This prevents clearing configuration while guard is still enabled
if (_isGuardEnabled(callingSafe)) {
revert TimelockGuard_GuardStillEnabled();
}
Comment thread
maurelian marked this conversation as resolved.

// Clear the configuration (guard should already be disabled by caller)
// set the timelock delay to 0 to clear the configuration
safeState.timelockDelay = 0;

// Reset the cancellation threshold to 0 (unconfigured state)
safeState.cancellationThreshold = 0;

// Get all pending transaction hashes
bytes32[] memory hashes = safeState.pendingTxHashes.values();

Comment thread
maurelian marked this conversation as resolved.
uint256 n = hashes.length <= 100 ? hashes.length : 100;

// Cancel all pending transactions up to 100
// It is very unlikely that there will be more than 100 pending transactions, so we can safely limit the
// number of iterations to 100 in order to prevent gas limit issues.
// If there are more than 100 pending transactions, then we emit an event to inform the user that some
// transactions were not cancelled.
for (uint256 i = 0; i < n; i++) {
safeState.pendingTxHashes.remove(hashes[i]);
safeState.scheduledTransactions[hashes[i]].state = TransactionState.Cancelled;
emit TransactionCancelled(callingSafe, hashes[i]);
}

if (hashes.length > 100) {
emit TransactionsNotCancelled(callingSafe, hashes.length - 100);
}
Comment thread
maurelian marked this conversation as resolved.
}

/// @notice Schedule a transaction for execution after the timelock delay.
/// @dev This function validates signatures in the exact same way as the Safe's own execTransaction function,
/// meaning that the same signatures used to schedule a transaction can be used to execute it later. This
Expand Down
Loading