Skip to content

WIP scheduled tx #17435

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.

Sign up for GitHub

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

Closed
maurelian wants to merge 2 commits into from
Closed

WIP scheduled tx #17435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c607c1
TimelockGuard: Add scheduleTransaction()
maurelian Sep 11, 2025
f68f19b
Add todo note
maurelian Sep 11, 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
97 changes: 78 additions & 19 deletions packages/contracts-bedrock/src/safe/TimelockGuard.sol
View file
Open in desktop
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 { GuardManager, Guard as IGuard } from "safe-contracts/base/GuardManager.sol";
import { ExecTransactionParams } from "src/safe/Types.sol";

// Interfaces
import { ISemver } from "interfaces/universal/ISemver.sol";
Expand All @@ -20,12 +21,21 @@ contract TimelockGuard is IGuard, ISemver {
uint256 timelockDelay;
}

/// @notice Scheduled transaction
struct ScheduledTransaction {
uint256 executionTime;
bool cancelled;
}

/// @notice Mapping from Safe address to its guard configuration
mapping(address => GuardConfig) public safeConfigs;

/// @notice Mapping from Safe address to its current cancellation threshold
mapping(address => uint256) public safeCancellationThreshold;

/// @notice Mapping from Safe and tx id to scheduled transaction.
mapping(Safe => mapping(bytes32 => ScheduledTransaction)) public scheduledTransactions;

/// @notice Error for when guard is not enabled for the Safe
error TimelockGuard_GuardNotEnabled();

Expand All @@ -38,12 +48,21 @@ contract TimelockGuard is IGuard, ISemver {
/// @notice Error for invalid timelock delay
error TimelockGuard_InvalidTimelockDelay();

/// @notice Error for when a transaction is already scheduled
error TimelockGuard_TransactionAlreadyScheduled();

/// @notice Emitted when a Safe configures the guard
event GuardConfigured(address indexed safe, uint256 timelockDelay);

/// @notice Emitted when a Safe clears the guard configuration
event GuardCleared(address indexed safe);

/// @notice Emitted when a transaction is scheduled for a Safe.
/// @param safe The Safe whose transaction is scheduled.
/// @param txId The identifier of the scheduled transaction (nonce-independent).
/// @param when The timestamp when execution becomes valid.
event TransactionScheduled(Safe indexed safe, bytes32 indexed txId, uint256 when);

/// @notice Semantic version.
/// @custom:semver 1.0.0
string public constant version = "1.0.0";
Expand Down Expand Up @@ -131,25 +150,65 @@ contract TimelockGuard is IGuard, ISemver {
return guard == address(this);
}

/// @notice Schedule a transaction for execution after the timelock delay
/// @dev Called by anyone using signatures from Safe owners - NOT IMPLEMENTED YET
function scheduleTransaction(
address,
address,
uint256,
bytes memory,
Enum.Operation,
uint256,
uint256,
uint256,
address,
address payable,
bytes memory
)
external
pure
{
// TODO: Implement
/// @notice Schedule a transaction for execution after the timelock delay.
/// @dev Minimal implementation: checks enabled+configured, uniqueness, cancellation, stores execution time and
/// emits.
/// @dev The txId is computed independent of Safe nonce using all exec params (with keccak(data)).
function scheduleTransaction(Safe _safe, uint256 _nonce, ExecTransactionParams memory _params) external {
// Check that this guard is enabled on the calling Safe
if (!_isGuardEnabled(address(_safe))) {
revert TimelockGuard_GuardNotEnabled();
}

// Get the encoded transaction data as defined in the Safe
// The format of the string returned is: "0x1901{domainSeparator}{safeTxHash}"
bytes memory txHashData = _safe.encodeTransactionData(
_params.to,
_params.value,
_params.data,
_params.operation,
_params.safeTxGas,
_params.baseGas,
_params.gasPrice,
_params.gasToken,
_params.refundReceiver,
_nonce
);

// Get the transaction hash and data as defined in the Safe
// This value is identical to keccak256(txHashData), but we prefer to use the Safe's own
// internal logic as it is more future-proof in case future versions of the Safe change
// the transaction hash derivation.
bytes32 txHash = _safe.getTransactionHash(
_params.to,
_params.value,
_params.data,
_params.operation,
_params.safeTxGas,
_params.baseGas,
_params.gasPrice,
_params.gasToken,
_params.refundReceiver,
_nonce
);

// Verify signatures using the Safe's signature checking logic
// This function call reverts if the signatures are invalid.
_safe.checkSignatures(txHash, txHashData, _params.signatures);

// Check if the transaction exists
// A transaction can only be scheduled once, regardless of whether it has been cancelled or not.
if (scheduledTransactions[_safe][txHash].executionTime != 0) {
revert TimelockGuard_TransactionAlreadyScheduled();
}
Copy link
Copy Markdown
Contributor

@alcueca alcueca Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is sufficient to ensure that the same signatures can't ever be used twice, because the data used to derive txHash is the same used to derive the signatures. You can't change txHash without having to change the signatures as well.


// Calculate the execution time
uint256 executionTime = block.timestamp + safeConfigs[address(_safe)].timelockDelay;

// Schedule the transaction
scheduledTransactions[_safe][txHash] = ScheduledTransaction({ executionTime: executionTime, cancelled: false });

emit TransactionScheduled(_safe, txHash, executionTime);
}

/// @notice Returns the list of all scheduled but not cancelled transactions for a given safe
Expand Down
19 changes: 19 additions & 0 deletions packages/contracts-bedrock/src/safe/Types.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import { Enum } from "safe-contracts/common/Enum.sol";

/// @notice Parameters for the Safe's execTransaction function
struct ExecTransactionParams {
address to;
uint256 value;
bytes data;
Enum.Operation operation;
uint256 safeTxGas;
uint256 baseGas;
uint256 gasPrice;
address gasToken;
address payable refundReceiver;
// TODO: Life might be easier if this was left out of the struct
bytes signatures;
}
2 changes: 2 additions & 0 deletions packages/contracts-bedrock/test/safe-tools/SafeTestTools.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ library SafeTestLib {
refundReceiver: refundReceiver,
_nonce: _nonce
});
console.log('txDataHash:');
console.logBytes32(txDataHash);
}

(v, r, s) = Vm(VM_ADDR).sign(pk, txDataHash);
Expand Down
109 changes: 94 additions & 15 deletions packages/contracts-bedrock/test/safe/TimelockGuard.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@
pragma solidity 0.8.15;

import { Test } from "forge-std/Test.sol";
import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol";
import { Enum } from "safe-contracts/common/Enum.sol";
import { GuardManager } from "safe-contracts/base/GuardManager.sol";
import { StorageAccessible } from "safe-contracts/common/StorageAccessible.sol";
import { ExecTransactionParams } from "src/safe/Types.sol";
import "test/safe-tools/SafeTestTools.sol";

import { console2 as console } from "forge-std/console2.sol";

import { TimelockGuard } from "src/safe/TimelockGuard.sol";

/// @title TimelockGuard_TestInit
Expand All @@ -17,6 +21,7 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {
// Events
event GuardConfigured(address indexed safe, uint256 timelockDelay);
event GuardCleared(address indexed safe);
event TransactionScheduled(Safe indexed safe, bytes32 indexed txId, uint256 when);

uint256 constant INIT_TIME = 10;
uint256 constant TIMELOCK_DELAY = 7 days;
Expand All @@ -26,9 +31,7 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {

TimelockGuard timelockGuard;
SafeInstance safeInstance;
SafeInstance safeInstance2;
address[] owners;
uint256[] ownerPKs;
SafeInstance unguardedSafe;

function setUp() public virtual {
vm.warp(INIT_TIME);
Expand All @@ -37,12 +40,14 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {
timelockGuard = new TimelockGuard();

// Create Safe owners
(address[] memory _owners, uint256[] memory _keys) = SafeTestLib.makeAddrsAndKeys("owners", NUM_OWNERS);
owners = _owners;
ownerPKs = _keys;
(, uint256[] memory keys) = SafeTestLib.makeAddrsAndKeys("owners", NUM_OWNERS);

// Set up Safe with owners
safeInstance = _setupSafe(ownerPKs, THRESHOLD);
safeInstance = _setupSafe(keys, THRESHOLD);

// Safe without guard enabled
// Reduce the threshold just to prevent a CREATE2 collision when deploying this safe.
unguardedSafe = _setupSafe(keys, THRESHOLD - 1);

// Enable the guard on the Safe
SafeTestLib.execTransaction(
Expand All @@ -54,6 +59,54 @@ contract TimelockGuard_TestInit is Test, SafeTestTools {
);
}

/// @notice Helper to create a dummy transaction with signatures and a tx hash
// TODO: separate into two functions: one for the params+hash, one for the signatures
function _getDummyTx() internal view returns (ExecTransactionParams memory, bytes32) {
// Get the nonce of the safe to sign
uint256 nonce = safeInstance.safe.nonce();

// Declare the dummy transaction params with an empty signature
ExecTransactionParams memory dummyTxParams = ExecTransactionParams({
to: address(0xabba),
value: 0,
data: hex"acdc",
operation: Enum.Operation.Call,
safeTxGas: 0,
baseGas: 0,
gasPrice: 0,
gasToken: address(0),
refundReceiver: payable(address(0)),
signatures: new bytes(0)
});

// Get the tx hash
bytes32 txHash;
{
txHash = safeInstance.safe.getTransactionHash({
to: dummyTxParams.to,
value: dummyTxParams.value,
data: dummyTxParams.data,
operation: dummyTxParams.operation,
safeTxGas: dummyTxParams.safeTxGas,
baseGas: dummyTxParams.baseGas,
gasPrice: dummyTxParams.gasPrice,
gasToken: dummyTxParams.gasToken,
refundReceiver: dummyTxParams.refundReceiver,
_nonce: nonce
});
}

// Sign the tx hash with the owners' private keys
for (uint256 i; i < THRESHOLD; ++i) {
(uint8 v, bytes32 r, bytes32 s) = vm.sign(safeInstance.ownerPKs[i], txHash);

// The signature format is a compact form of: {bytes32 r}{bytes32 s}{uint8 v}
dummyTxParams.signatures = bytes.concat(dummyTxParams.signatures, abi.encodePacked(r, s, v));
}

return (dummyTxParams, txHash);
}

/// @notice Helper to configure the TimelockGuard for a Safe
function _configureGuard(SafeInstance memory _safe, uint256 _delay) internal {
SafeTestLib.execTransaction(
Expand Down Expand Up @@ -103,10 +156,6 @@ contract TimelockGuard_ConfigureTimelockGuard_Test is TimelockGuard_TestInit {
}

function test_configureTimelockGuard_revertsIfGuardNotEnabled_reverts() external {
// Create a safe without enabling the guard
// Reduce the threshold just to prevent a CREATE2 collision when deploying this safe.
SafeInstance memory unguardedSafe = _setupSafe(ownerPKs, THRESHOLD - 1);

vm.expectRevert(TimelockGuard.TimelockGuard_GuardNotEnabled.selector);
vm.prank(address(unguardedSafe.safe));
timelockGuard.configureTimelockGuard(TIMELOCK_DELAY);
Expand Down Expand Up @@ -192,14 +241,11 @@ contract TimelockGuard_ClearTimelockGuard_Test is TimelockGuard_TestInit {
/// @notice Tests for cancellationThreshold function
contract TimelockGuard_CancellationThreshold_Test is TimelockGuard_TestInit {
function test_cancellationThreshold_returnsZeroIfGuardNotEnabled_succeeds() external {
// Safe without guard enabled should return 0
SafeInstance memory unguardedSafe = _setupSafe(ownerPKs, THRESHOLD - 1);

uint256 threshold = timelockGuard.cancellationThreshold(address(unguardedSafe.safe));
assertEq(threshold, 0);
}

function test_cancellationThreshold_returnsZeroIfGuardNotConfigured_succeeds() external {
function test_cancellationThreshold_returnsZeroIfGuardNotConfigured_succeeds() external view {
// Safe with guard enabled but not configured should return 0
uint256 threshold = timelockGuard.cancellationThreshold(address(safeInstance.safe));
assertEq(threshold, 0);
Expand All @@ -217,3 +263,36 @@ contract TimelockGuard_CancellationThreshold_Test is TimelockGuard_TestInit {
// Note: Testing increment/decrement behavior will require scheduleTransaction,
// cancelTransaction and execution functions to be implemented first
}

/// @title TimelockGuard_ScheduleTransaction_Test
/// @notice Tests for scheduleTransaction function
contract TimelockGuard_ScheduleTransaction_Test is TimelockGuard_TestInit {
function setUp() public override {
super.setUp();
_configureGuard(safeInstance, TIMELOCK_DELAY);
}

function test_scheduleTransaction_succeeds() public {
(ExecTransactionParams memory dummyTxParams, bytes32 txHash) = _getDummyTx();

vm.expectEmit(true, true, true, true);
emit TransactionScheduled(safeInstance.safe, txHash, INIT_TIME + TIMELOCK_DELAY);
timelockGuard.scheduleTransaction(safeInstance.safe, safeInstance.safe.nonce(), dummyTxParams);
}

function test_scheduleTransaction_reschedulingIdenticalTransaction_reverts() external {
(ExecTransactionParams memory dummyTxParams,) = _getDummyTx();
timelockGuard.scheduleTransaction(safeInstance.safe, safeInstance.safe.nonce(), dummyTxParams);

vm.expectRevert(TimelockGuard.TimelockGuard_TransactionAlreadyScheduled.selector);
timelockGuard.scheduleTransaction(safeInstance.safe, safeInstance.safe.nonce(), dummyTxParams);
}

function test_scheduleTransaction_identicalPreviouslyCancelled_reverts() external { }

function test_scheduleTransaction_guardNotEnabled_reverts() external { }

function test_scheduleTransaction_guardNotConfigured_reverts() external { }

function test_scheduleTransaction_canScheduleIdenticalWithSalt_succeeds() external { }
}