diff --git a/packages/contracts-bedrock/interfaces/safe/ITimelockGuard.sol b/packages/contracts-bedrock/interfaces/safe/ITimelockGuard.sol new file mode 100644 index 0000000000000..ccf454985a90d --- /dev/null +++ b/packages/contracts-bedrock/interfaces/safe/ITimelockGuard.sol @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.4; + +library Enum { + type Operation is uint8; +} + +interface ITimelockGuard { + enum TransactionState { + NotScheduled, + Pending, + Cancelled, + Executed + } + struct ScheduledTransaction { + uint256 executionTime; + TransactionState state; + ExecTransactionParams params; + } + + struct ExecTransactionParams { + address to; + uint256 value; + bytes data; + Enum.Operation operation; + uint256 safeTxGas; + uint256 baseGas; + uint256 gasPrice; + address gasToken; + address payable refundReceiver; + } + + error TimelockGuard_GuardNotConfigured(); + error TimelockGuard_GuardNotEnabled(); + error TimelockGuard_GuardStillEnabled(); + error TimelockGuard_InvalidTimelockDelay(); + error TimelockGuard_TransactionAlreadyCancelled(); + error TimelockGuard_TransactionAlreadyScheduled(); + error TimelockGuard_TransactionNotScheduled(); + error TimelockGuard_TransactionNotReady(); + error TimelockGuard_TransactionAlreadyExecuted(); + error TimelockGuard_InvalidVersion(); + + event CancellationThresholdUpdated(address indexed safe, uint256 oldThreshold, uint256 newThreshold); + event GuardConfigured(address indexed safe, uint256 timelockDelay); + event TransactionCancelled(address indexed safe, bytes32 indexed txHash); + event TransactionScheduled(address indexed safe, bytes32 indexed txHash, uint256 executionTime); + event TransactionExecuted(address indexed safe, bytes32 txHash); + event Message(string message); + + function cancelTransaction(address _safe, bytes32 _txHash, uint256 _nonce, bytes memory _signatures) external; + function signCancellation(bytes32 _txHash) external; + function cancellationThreshold(address _safe) external view returns (uint256); + function checkTransaction( + address _to, + uint256 _value, + bytes memory _data, + Enum.Operation _operation, + uint256 _safeTxGas, + uint256 _baseGas, + uint256 _gasPrice, + address _gasToken, + address payable _refundReceiver, + bytes memory _signatures, + address _msgSender + ) + external; + function checkAfterExecution(bytes32, bool) external; + function configureTimelockGuard(uint256 _timelockDelay) external; + function scheduledTransaction( + address _safe, + bytes32 _txHash + ) + external + view + returns (ScheduledTransaction memory); + function safeConfigs(address) external view returns (uint256 timelockDelay); + function scheduleTransaction( + address _safe, + uint256 _nonce, + ExecTransactionParams memory _params, + bytes memory _signatures + ) + external; + function version() external view returns (string memory); + function timelockConfiguration(address _safe) external view returns (uint256 timelockDelay); + function maxCancellationThreshold(address _safe) external view returns (uint256); + function pendingTransactions(address _safe) + external + view + returns (ScheduledTransaction[] memory); +} diff --git a/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml b/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml index dd174c68b582e..6c332b4e8a321 100644 --- a/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml +++ b/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml @@ -92,5 +92,6 @@ contracts = [ "OptimismPortal2_MigrateLiquidity_Test", # Interop tests hosted in the OptimismPortal2 test file "OptimismPortal2_MigrateToSuperRoots_Test", # Interop tests hosted in the OptimismPortal2 test file "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 ] diff --git a/packages/contracts-bedrock/scripts/checks/test-validation/main.go b/packages/contracts-bedrock/scripts/checks/test-validation/main.go index d752e2566243d..776e769ac3fc4 100644 --- a/packages/contracts-bedrock/scripts/checks/test-validation/main.go +++ b/packages/contracts-bedrock/scripts/checks/test-validation/main.go @@ -194,9 +194,12 @@ func checkTestStructure(artifact *solc.ForgeArtifact) []error { func checkTestMethodName(artifact *solc.ForgeArtifact, contractName string, functionName string, _ string) []error { // Check for uncategorized test pattern - if functionName == "Uncategorized" { - // Pattern: _Uncategorized_Test - return nil + allowedFunctionNames := []string{"Uncategorized", "Integration"} + for _, allowed := range allowedFunctionNames { + if functionName == allowed { + // Pattern: _Uncategorized_Test or _Integration_Test + return nil + } } // Pattern: __Test - validate function exists if !checkFunctionExists(artifact, functionName) { @@ -250,6 +253,11 @@ func checkSrcPath(artifact *solc.ForgeArtifact) bool { // Validates that contract name matches the file path func checkContractNameFilePath(artifact *solc.ForgeArtifact) bool { for filePath, contractName := range artifact.Metadata.Settings.CompilationTarget { + + if isExcludedTest(contractName) { + continue + } + // Split contract name to get the base contract name (before first underscore) contractParts := strings.Split(contractName, "_") // Split file path to get individual path components diff --git a/packages/contracts-bedrock/snapshots/abi/TimelockGuard.json b/packages/contracts-bedrock/snapshots/abi/TimelockGuard.json new file mode 100644 index 0000000000000..15145753d43b0 --- /dev/null +++ b/packages/contracts-bedrock/snapshots/abi/TimelockGuard.json @@ -0,0 +1,623 @@ +[ + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + }, + { + "internalType": "bytes32", + "name": "_txHash", + "type": "bytes32" + }, + { + "internalType": "uint256", + "name": "_nonce", + "type": "uint256" + }, + { + "internalType": "bytes", + "name": "_signatures", + "type": "bytes" + } + ], + "name": "cancelTransaction", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + } + ], + "name": "cancellationThreshold", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "bytes32", + "name": "_txHash", + "type": "bytes32" + }, + { + "internalType": "bool", + "name": "_success", + "type": "bool" + } + ], + "name": "checkAfterExecution", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "_to", + "type": "address" + }, + { + "internalType": "uint256", + "name": "_value", + "type": "uint256" + }, + { + "internalType": "bytes", + "name": "_data", + "type": "bytes" + }, + { + "internalType": "enum Enum.Operation", + "name": "_operation", + "type": "uint8" + }, + { + "internalType": "uint256", + "name": "_safeTxGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "_baseGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "_gasPrice", + "type": "uint256" + }, + { + "internalType": "address", + "name": "_gasToken", + "type": "address" + }, + { + "internalType": "address payable", + "name": "_refundReceiver", + "type": "address" + }, + { + "internalType": "bytes", + "name": "", + "type": "bytes" + }, + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "checkTransaction", + "outputs": [], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "_timelockDelay", + "type": "uint256" + } + ], + "name": "configureTimelockGuard", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + } + ], + "name": "maxCancellationThreshold", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + } + ], + "name": "pendingTransactions", + "outputs": [ + { + "components": [ + { + "internalType": "uint256", + "name": "executionTime", + "type": "uint256" + }, + { + "internalType": "enum TimelockGuard.TransactionState", + "name": "state", + "type": "uint8" + }, + { + "components": [ + { + "internalType": "address", + "name": "to", + "type": "address" + }, + { + "internalType": "uint256", + "name": "value", + "type": "uint256" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + }, + { + "internalType": "enum Enum.Operation", + "name": "operation", + "type": "uint8" + }, + { + "internalType": "uint256", + "name": "safeTxGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "baseGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "gasPrice", + "type": "uint256" + }, + { + "internalType": "address", + "name": "gasToken", + "type": "address" + }, + { + "internalType": "address payable", + "name": "refundReceiver", + "type": "address" + } + ], + "internalType": "struct TimelockGuard.ExecTransactionParams", + "name": "params", + "type": "tuple" + } + ], + "internalType": "struct TimelockGuard.ScheduledTransaction[]", + "name": "", + "type": "tuple[]" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + }, + { + "internalType": "uint256", + "name": "_nonce", + "type": "uint256" + }, + { + "components": [ + { + "internalType": "address", + "name": "to", + "type": "address" + }, + { + "internalType": "uint256", + "name": "value", + "type": "uint256" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + }, + { + "internalType": "enum Enum.Operation", + "name": "operation", + "type": "uint8" + }, + { + "internalType": "uint256", + "name": "safeTxGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "baseGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "gasPrice", + "type": "uint256" + }, + { + "internalType": "address", + "name": "gasToken", + "type": "address" + }, + { + "internalType": "address payable", + "name": "refundReceiver", + "type": "address" + } + ], + "internalType": "struct TimelockGuard.ExecTransactionParams", + "name": "_params", + "type": "tuple" + }, + { + "internalType": "bytes", + "name": "_signatures", + "type": "bytes" + } + ], + "name": "scheduleTransaction", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + }, + { + "internalType": "bytes32", + "name": "_txHash", + "type": "bytes32" + } + ], + "name": "scheduledTransaction", + "outputs": [ + { + "components": [ + { + "internalType": "uint256", + "name": "executionTime", + "type": "uint256" + }, + { + "internalType": "enum TimelockGuard.TransactionState", + "name": "state", + "type": "uint8" + }, + { + "components": [ + { + "internalType": "address", + "name": "to", + "type": "address" + }, + { + "internalType": "uint256", + "name": "value", + "type": "uint256" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + }, + { + "internalType": "enum Enum.Operation", + "name": "operation", + "type": "uint8" + }, + { + "internalType": "uint256", + "name": "safeTxGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "baseGas", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "gasPrice", + "type": "uint256" + }, + { + "internalType": "address", + "name": "gasToken", + "type": "address" + }, + { + "internalType": "address payable", + "name": "refundReceiver", + "type": "address" + } + ], + "internalType": "struct TimelockGuard.ExecTransactionParams", + "name": "params", + "type": "tuple" + } + ], + "internalType": "struct TimelockGuard.ScheduledTransaction", + "name": "", + "type": "tuple" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "bytes32", + "name": "", + "type": "bytes32" + } + ], + "name": "signCancellation", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract GnosisSafe", + "name": "_safe", + "type": "address" + } + ], + "name": "timelockConfiguration", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "version", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "contract GnosisSafe", + "name": "safe", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "oldThreshold", + "type": "uint256" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "newThreshold", + "type": "uint256" + } + ], + "name": "CancellationThresholdUpdated", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "contract GnosisSafe", + "name": "safe", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "timelockDelay", + "type": "uint256" + } + ], + "name": "GuardConfigured", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "string", + "name": "message", + "type": "string" + } + ], + "name": "Message", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "contract GnosisSafe", + "name": "safe", + "type": "address" + }, + { + "indexed": true, + "internalType": "bytes32", + "name": "txHash", + "type": "bytes32" + } + ], + "name": "TransactionCancelled", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "contract GnosisSafe", + "name": "safe", + "type": "address" + }, + { + "indexed": false, + "internalType": "bytes32", + "name": "txHash", + "type": "bytes32" + } + ], + "name": "TransactionExecuted", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "contract GnosisSafe", + "name": "safe", + "type": "address" + }, + { + "indexed": true, + "internalType": "bytes32", + "name": "txHash", + "type": "bytes32" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "executionTime", + "type": "uint256" + } + ], + "name": "TransactionScheduled", + "type": "event" + }, + { + "inputs": [], + "name": "SemverComp_InvalidSemverParts", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_GuardNotConfigured", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_GuardNotEnabled", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_InvalidTimelockDelay", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_InvalidVersion", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_TransactionAlreadyCancelled", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_TransactionAlreadyExecuted", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_TransactionAlreadyScheduled", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_TransactionNotReady", + "type": "error" + }, + { + "inputs": [], + "name": "TimelockGuard_TransactionNotScheduled", + "type": "error" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index eb5f4a75cbf85..4c4a77f3508f4 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -211,6 +211,10 @@ "initCodeHash": "0x4679b41e5648a955a883efd0271453c8b13ff4846f853d372527ebb1e0905ab5", "sourceCodeHash": "0xd3084fb5446782cb6d0adb4278ef0a12c418dd538b4b14b90407b971b44cc35b" }, + "src/safe/TimelockGuard.sol:TimelockGuard": { + "initCodeHash": "0x1f8188872de93ce59e8f0bd415d4fbf30209bc668c09623f61d6fe592eee895a", + "sourceCodeHash": "0x0dada93f051d29dabbb6de3e1c1ece14b95cd20dc854454926d19ea1ebcae436" + }, "src/universal/OptimismMintableERC20.sol:OptimismMintableERC20": { "initCodeHash": "0x3c85eed0d017dca8eda6396aa842ddc12492587b061e8c756a8d32c4610a9658", "sourceCodeHash": "0x7023665d461f173417d932b55010b8f6c34f2bbaf56cfe4e1b15862c08cbcaac" diff --git a/packages/contracts-bedrock/snapshots/storageLayout/TimelockGuard.json b/packages/contracts-bedrock/snapshots/storageLayout/TimelockGuard.json new file mode 100644 index 0000000000000..97c754bfc8c5a --- /dev/null +++ b/packages/contracts-bedrock/snapshots/storageLayout/TimelockGuard.json @@ -0,0 +1,9 @@ +[ + { + "bytes": "32", + "label": "_safeState", + "offset": 0, + "slot": "0", + "type": "mapping(contract GnosisSafe => struct TimelockGuard.SafeState)" + } +] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/safe/TimelockGuard.sol b/packages/contracts-bedrock/src/safe/TimelockGuard.sol new file mode 100644 index 0000000000000..26dce92ce472e --- /dev/null +++ b/packages/contracts-bedrock/src/safe/TimelockGuard.sol @@ -0,0 +1,615 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +// Safe +import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol"; +import { Enum } from "safe-contracts/common/Enum.sol"; +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"; + +// Interfaces +import { ISemver } from "interfaces/universal/ISemver.sol"; + +/// @title TimelockGuard +/// @notice This guard provides timelock functionality for Safe transactions +/// @dev This is a singleton contract, any Safe on the network can use this guard to enforce a timelock delay, and +/// allow a subset of signers to cancel a transaction if they do not agree with the execution. This provides +/// significant security improvements over the Safe's default execution mechanism, which will allow any transaction +/// to be executed as long as it is fully signed, and with no mechanism for revealing the existence of said +/// signatures. +/// Usage: +/// In order to use this guard, the Safe must first enable it using Safe.setGuard(), and then configure it +/// by calling TimelockGuard.configureTimelockGuard(). +/// Scheduling and executing transactions: +/// Once enabled and configured, all transactions executed by the Safe's execTransaction() function will revert, +/// unless the transaction has first been scheduled by calling scheduleTransaction() on this contract. Because +/// scheduleTransaction() uses the Safe's own signature verification logic, the same signatures used +/// to execute a transaction can be used to schedule it. +/// Note: this guard does not apply a delay to transactions executed by modules which are installed on the Safe. +/// Cancelling transactions: +/// Once a transaction has been scheduled, so long as it has not already been executed, it can be +/// cancelled by calling cancelTransaction() on this contract. +/// This mechanism allows for a subset of signers to cancel a transaction if they do not agree with the execution. +/// As an 'anti-griefing' mechanism, the cancellation threshold (the number of signatures required to cancel a +/// transaction) starts at 1, and is automatically increased by 1 after each cancellation. +/// The cancellation threshold is reset to 1 after any transaction is executed successfully. +/// Safe Version Compatibility: +/// This guard is compatible with Safe versions 1.3.0 and higher. Earlier versions of the Safe do not expose +/// the checkSignatures or checkNSignatures functions required by this guard. +/// Threats Mitigated and Integration With LivenessModule: +/// This Guard is designed to protect against a number of well-defined scenarios, defined on +/// the two axes of amount of keys compromised, and type of compromise. +/// For scenarios where the keys compromised don't amount to a blocking threshold (the number of signers who must +/// refuse to sign a transaction in order to block it from being executed), regular transactions from the +/// multisig for removal or rotation is the preferred solution. +/// For scenarios where the keys compromised are at least a blocking threshold, but not as much as quorum, the +/// LivenessModule would be used. If there is a quorum of absent keys, but no significant malicious control, the +/// LivenessModule would also be used. +/// The TimelockGuard acts when there is malicious control of a quorum of keys. If the control is temporary, for +/// example by phishing a single set of signatures, then the TimelockGuard's cancellation is enough to stop the +/// attack entirely. If the malicious control would be permanent, then the TimelockGuard will buy some time to +/// execute remediations external to the compromised safe. +/// The following table summarizes the various scenarios and the course of action to take in each case. +/// +---------------------------------------------------------------------------+ +/// | Course of action when X Number of keys... | +/// +-------------------------------------------------------------------------------------------------+ +/// | | ... are Absent | ... are Maliciously Controlled | +/// | X Number of keys | (Honest signers cannot sign) | (Malicious signers can sign) | +/// +-------------------------------------------------------------------------------------------------+ +/// | 1+ | swapOwner | swapOwner | +/// +-------------------------------------------------------------------------------------------------+ +/// | Blocking Threshold+ | challenge + | challenge + | +/// | | changeOwnershipToFallback | changeOwnershipToFallback | +/// +-------------------------------------------------------------------------------------------------+ +/// | Quorum+ | challenge + | cancelTransaction | +/// | | changeOwnershipToFallback | | +/// +-------------------------------------------------------------------------------------------------+ +contract TimelockGuard is IGuard, ISemver { + using EnumerableSet for EnumerableSet.Bytes32Set; + + /// @notice Allowed states of a transaction + enum TransactionState { + NotScheduled, + Pending, + Cancelled, + Executed + } + + /// @notice Scheduled transaction + /// @custom:field executionTime The timestamp when execution becomes valid. + /// @custom:field state The state of the transaction. + /// @custom:field params The parameters of the transaction. + struct ScheduledTransaction { + uint256 executionTime; + TransactionState state; + ExecTransactionParams params; + } + + /// @notice Parameters for the Safe's execTransaction function + /// @custom:field to The address of the contract to call. + /// @custom:field value The value to send with the transaction. + /// @custom:field data The data to send with the transaction. + /// @custom:field operation The operation to perform with the transaction. + /// @custom:field safeTxGas The gas to use for the transaction. + /// @custom:field baseGas The base gas to use for the transaction. + /// @custom:field gasPrice The gas price to use for the transaction. + /// @custom:field gasToken The token to use for the transaction. + /// @custom:field refundReceiver The address to receive the refund for the transaction. + struct ExecTransactionParams { + address to; + uint256 value; + bytes data; + Enum.Operation operation; + uint256 safeTxGas; + uint256 baseGas; + uint256 gasPrice; + address gasToken; + address payable refundReceiver; + } + + /// @notice Aggregated state for each Safe using this guard. + /// @dev We have chosen for operational reasons to keep a list of pending transactions that can be easily retrieved + /// via a function call. This is done by maintaining a separate EnumerableSet with the hashes of pending + /// transactions. Transactions in the enumerable set need to be updated along with updates to the + /// ScheduledTransactions mapping. + struct SafeState { + uint256 timelockDelay; + uint256 cancellationThreshold; + mapping(bytes32 => ScheduledTransaction) scheduledTransactions; + EnumerableSet.Bytes32Set pendingTxHashes; + } + + /// @notice Mapping from Safe address to its timelock guard state. + mapping(Safe => SafeState) internal _safeState; + + /// @notice Semantic version. + /// @custom:semver 1.0.0 + string public constant version = "1.0.0"; + + /// @notice Error for when guard is not enabled for the Safe + error TimelockGuard_GuardNotEnabled(); + + /// @notice Error for when Safe is not configured for this guard + error TimelockGuard_GuardNotConfigured(); + + /// @notice Error for invalid timelock delay + error TimelockGuard_InvalidTimelockDelay(); + + /// @notice Error for when a transaction is already scheduled + error TimelockGuard_TransactionAlreadyScheduled(); + + /// @notice Error for when a transaction is already cancelled + error TimelockGuard_TransactionAlreadyCancelled(); + + /// @notice Error for when a transaction is not scheduled + error TimelockGuard_TransactionNotScheduled(); + + /// @notice Error for when a transaction is not ready to execute (timelock delay not passed) + error TimelockGuard_TransactionNotReady(); + + /// @notice Error for when a transaction has already been executed + error TimelockGuard_TransactionAlreadyExecuted(); + + /// @notice Error for when the contract is not at least version 1.3.0 + error TimelockGuard_InvalidVersion(); + + /// @notice Emitted when a Safe configures the guard + /// @param safe The Safe whose guard is configured. + /// @param timelockDelay The timelock delay in seconds. + event GuardConfigured(Safe indexed safe, uint256 timelockDelay); + + /// @notice Emitted when a transaction is scheduled for a Safe. + /// @param safe The Safe whose transaction is scheduled. + /// @param txHash The identifier of the scheduled transaction (nonce-independent). + /// @param executionTime The timestamp when execution becomes valid. + event TransactionScheduled(Safe indexed safe, bytes32 indexed txHash, uint256 executionTime); + + /// @notice Emitted when a transaction is cancelled for a Safe. + /// @param safe The Safe whose transaction is cancelled. + /// @param txHash The identifier of the cancelled transaction (nonce-independent). + event TransactionCancelled(Safe indexed safe, bytes32 indexed txHash); + + /// @notice Emitted when the cancellation threshold is updated + /// @param safe The Safe whose cancellation threshold is updated. + /// @param oldThreshold The old cancellation threshold. + /// @param newThreshold The new cancellation threshold. + event CancellationThresholdUpdated(Safe indexed safe, uint256 oldThreshold, uint256 newThreshold); + + /// @notice Emitted when a transaction is executed for a Safe. + /// @param safe The Safe whose transaction is executed. + /// @param txHash The identifier of the executed transaction (nonce-independent). + event TransactionExecuted(Safe indexed safe, bytes32 txHash); + + /// @notice Used to emit a message, primarily to ensure that the cancelTransaction function is + /// is not labelled as view so that it is treated as a state-changing function. + event Message(string message); + + //////////////////////////////////////////////////////////////// + // Internal View Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Returns the blocking threshold, which is defined as the minimum number of owners that must coordinate to + /// block a transaction from being executed by refusing to sign. + /// @param _safe The Safe address to query + /// @return The current blocking threshold + function _blockingThreshold(Safe _safe) internal view returns (uint256) { + return _safe.getOwners().length - _safe.getThreshold() + 1; + } + + /// @notice Internal helper to get the guard address from a Safe + /// @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)); + return guard == address(this); + } + + //////////////////////////////////////////////////////////////// + // External View Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Returns the cancellation threshold for a given safe + /// @param _safe The Safe address to query + /// @return The current cancellation threshold + function cancellationThreshold(Safe _safe) public view returns (uint256) { + // Return 0 if guard is not enabled + if (!_isGuardEnabled(_safe)) { + return 0; + } + + return _safeState[_safe].cancellationThreshold; + } + + /// @notice Returns the maximum cancellation threshold for a given safe + /// @dev The cancellation threshold must be capped in order to preserve the ability of honest users to cancel + /// malicious transactions. The rationale for the calculation of the maximum cancellation threshold is as + /// follows: + /// If the quorum is lower, then it is used as the maximum cancellation threshold, + /// so that even if an attacker has _joint control_ of a quorum of keys, the honest users can still + /// indefinitely cancel a malicious transaction. + /// If the blocking threshold is lower, then it is used as the maximum cancellation threshold, so that if an + /// attacker has less than a quorum of keys, honest users can still remove an attacker from the Safe by + /// refusing to respond to a malicious transaction. + /// @param _safe The Safe address to query + /// @return The maximum cancellation threshold + function maxCancellationThreshold(Safe _safe) public view returns (uint256) { + uint256 blockingThreshold = _blockingThreshold(_safe); + uint256 quorum = _safe.getThreshold(); + // Return the minimum of the blocking threshold and the quorum + return (blockingThreshold < quorum ? blockingThreshold : quorum); + } + + /// @notice Returns the timelock delay for a given Safe + /// @param _safe The Safe address to query + /// @return The timelock delay in seconds + function timelockConfiguration(Safe _safe) public view returns (uint256) { + return _safeState[_safe].timelockDelay; + } + + /// @notice Returns the scheduled transaction for a given Safe and tx hash + /// @dev This function is necessary to properly expose the scheduledTransactions mapping, as + /// simply making the mapping public will return a tuple instead of a struct. + function scheduledTransaction(Safe _safe, bytes32 _txHash) public view returns (ScheduledTransaction memory) { + return _safeState[_safe].scheduledTransactions[_txHash]; + } + + /// @notice Returns the list of all scheduled but not cancelled or executed transactions for + /// for a given safe + /// @dev WARNING: This operation will copy the entire set of pending transactions to memory, + /// which can be quite expensive. This is designed only to be used by view accessors that are + /// queried without any gas fees. Developers should keep in mind that this function has an + /// unbounded cost, and using it as part of a state-changing function may render the function + /// uncallable if the set grows to a point where copying to memory consumes too much gas to fit + /// in a block. + /// @return List of pending transaction hashes + function pendingTransactions(Safe _safe) external view returns (ScheduledTransaction[] memory) { + SafeState storage safeState = _safeState[_safe]; + + // Get the list of pending transaction hashes + bytes32[] memory hashes = safeState.pendingTxHashes.values(); + + // We want to provide the caller with the full parameters of each pending transaction, but mappings are not + // iterable, so we use the enumerable set of pending transaction hashes to retrieve the ScheduledTransaction + // struct for each hash, and then return an array of the ScheduledTransaction structs. + ScheduledTransaction[] memory scheduled = new ScheduledTransaction[](hashes.length); + for (uint256 i = 0; i < hashes.length; i++) { + scheduled[i] = safeState.scheduledTransactions[hashes[i]]; + } + return scheduled; + } + + //////////////////////////////////////////////////////////////// + // Guard Interface Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Implementation of IGuard interface.Called by the Safe before executing a transaction + /// @dev This function is used to check that the transaction has been scheduled and is ready to execute. + /// It only reads the state of the contract, and potentially reverts in order to protect against execution of + /// unscheduled, early or cancelled transactions. + function checkTransaction( + address _to, + uint256 _value, + bytes memory _data, + Enum.Operation _operation, + uint256 _safeTxGas, + uint256 _baseGas, + uint256 _gasPrice, + address _gasToken, + address payable _refundReceiver, + bytes memory, /* signatures */ + address /* msgSender */ + ) + external + view + override + { + Safe callingSafe = Safe(payable(msg.sender)); + + if (_safeState[callingSafe].timelockDelay == 0) { + // We return immediately. This is important in order to allow a Safe which has the + // guard set, but not configured, to complete the setup process. + + // It is also just a reasonable thing to do, since an unconfigured Safe must have a + // delay of zero. + return; + } + + // Get the nonce of the Safe for the transaction being executed, + // since the Safe's nonce is incremented before the transaction is executed, + // we must subtract 1. + uint256 nonce = callingSafe.nonce() - 1; + + // Get the transaction hash from the Safe's getTransactionHash function + bytes32 txHash = callingSafe.getTransactionHash( + _to, _value, _data, _operation, _safeTxGas, _baseGas, _gasPrice, _gasToken, _refundReceiver, nonce + ); + + // Get the scheduled transaction + ScheduledTransaction storage scheduledTx = _safeState[callingSafe].scheduledTransactions[txHash]; + + // Check if the transaction was cancelled + if (scheduledTx.state == TransactionState.Cancelled) { + revert TimelockGuard_TransactionAlreadyCancelled(); + } + + // Check if the transaction has already been executed + // Note: this is of course enforced by the Safe itself, but we check it here for + // completeness + if (scheduledTx.state == TransactionState.Executed) { + revert TimelockGuard_TransactionAlreadyExecuted(); + } + + // Check if the transaction has been scheduled + if (scheduledTx.state == TransactionState.NotScheduled) { + revert TimelockGuard_TransactionNotScheduled(); + } + + // Check if the timelock delay has passed + if (scheduledTx.executionTime > block.timestamp) { + revert TimelockGuard_TransactionNotReady(); + } + } + + /// @notice Implementation of IGuard interface. Called by the Safe after executing a transaction + /// @dev This function is used to update the state of the contract after the transaction has been executed. + /// Although making state changes here is a violation of the Checks-Effects-Interactions pattern, it + /// safe to do in this case because we trust that the Safe does not enable arbitrary calls without + /// proper authorization checks. + function checkAfterExecution(bytes32 _txHash, bool _success) external override { + Safe callingSafe = Safe(payable(msg.sender)); + // If the timelock delay is zero, we return immediately. + // This is important in order to allow a Safe which has the guard set, but not configured, + // to complete the setup process. + // It is also just a reasonable thing to do, since an unconfigured Safe must have a delay of zero, and so + // we do not expect the transaction to have been scheduled. + if (_safeState[callingSafe].timelockDelay == 0) { + return; + } + + // If the transaction failed, then we return early and leave the transaction in its current state, + // which allows the transaction to be retried. + // This is consistent with the Safe's own behaviour, which does not increment the nonce if the + // call fails. + if (!_success) { + return; + } + + ScheduledTransaction storage scheduledTx = _safeState[callingSafe].scheduledTransactions[_txHash]; + + // Set the transaction as executed + scheduledTx.state = TransactionState.Executed; + _safeState[callingSafe].pendingTxHashes.remove(_txHash); + + // Reset the cancellation threshold + _resetCancellationThreshold(callingSafe); + + emit TransactionExecuted(callingSafe, _txHash); + } + + //////////////////////////////////////////////////////////////// + // Internal State-Changing Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Increase the cancellation threshold for a safe + /// @dev This function must be called only once and only when calling cancel + /// @param _safe The Safe address to increase the cancellation threshold for. + function _increaseCancellationThreshold(Safe _safe) internal { + SafeState storage safeState = _safeState[_safe]; + + if (safeState.cancellationThreshold < maxCancellationThreshold(_safe)) { + uint256 oldThreshold = safeState.cancellationThreshold; + safeState.cancellationThreshold++; + emit CancellationThresholdUpdated(_safe, oldThreshold, safeState.cancellationThreshold); + } + } + + /// @notice Reset the cancellation threshold for a safe + /// @dev This function must be called only once and only when calling checkAfterExecution + /// @param _safe The Safe address to reset the cancellation threshold for. + function _resetCancellationThreshold(Safe _safe) internal { + SafeState storage safeState = _safeState[_safe]; + uint256 oldThreshold = safeState.cancellationThreshold; + safeState.cancellationThreshold = 1; + emit CancellationThresholdUpdated(_safe, oldThreshold, 1); + } + + //////////////////////////////////////////////////////////////// + // External State-Changing Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Configure the contract as a timelock guard by setting the timelock delay + /// @dev This function is only callable by the Safe itself. + /// Requiring a call from the Safe itself (rather than accepting signatures directly as in cancelTransaction()) + /// is important to ensure that maliciously gathered signatures will not be able to instantly reconfigure + /// the delay to zero. This function does not check that the guard is enabled on the Safe, the recommended + /// approach is to atomically enable the guard and configure the delay in a single batched transaction. + /// @param _timelockDelay The timelock delay in seconds (0 to clear configuration) + function configureTimelockGuard(uint256 _timelockDelay) external { + // Record the calling Safe + Safe callingSafe = Safe(payable(msg.sender)); + + // Check that the contract is at least version 1.3.0 + // Prior to version 1.3.0, checkSignatures() was not exposed as a public function, so we need to check the + // version otherwise the safe will be bricked. + if (SemverComp.lt(callingSafe.VERSION(), "1.3.0")) { + revert TimelockGuard_InvalidVersion(); + } + + // Validate timelock delay - must not be longer than 1 year + if (_timelockDelay > 365 days) { + revert TimelockGuard_InvalidTimelockDelay(); + } + + // Store the timelock delay for this safe + _safeState[callingSafe].timelockDelay = _timelockDelay; + + // Initialize (or reset) the cancellation threshold to 1. + _resetCancellationThreshold(callingSafe); + emit GuardConfigured(callingSafe, _timelockDelay); + } + + /// @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 + /// maintains compatibility with existing signature generation tools. Owners can use any method to sign the + /// a transaction, including signing with a private key, calling the Safe's approveHash function, or EIP1271 + /// contract signatures. + /// @param _safe The Safe address to schedule the transaction for. + /// @param _nonce The nonce of the Safe for the transaction being scheduled. + /// @param _params The parameters of the transaction being scheduled. + /// @param _signatures The signatures of the owners who are scheduling the transaction. + function scheduleTransaction( + Safe _safe, + uint256 _nonce, + ExecTransactionParams memory _params, + bytes memory _signatures + ) + external + { + // Check that this guard is enabled on the calling Safe + if (!_isGuardEnabled(_safe)) { + revert TimelockGuard_GuardNotEnabled(); + } + + // Check that the guard has been configured for the Safe + if (_safeState[_safe].timelockDelay == 0) { + revert TimelockGuard_GuardNotConfigured(); + } + + // 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 + ); + + // Check if the transaction exists + // A transaction can only be scheduled once, regardless of whether it has been cancelled or not, + // as otherwise an observer could reuse the same signatures to either: + // 1. Reschedule a transaction after it has been cancelled + // 2. Reschedule a pending transaction, which would update the execution time thus extending the delay + // for the original transaction. + if (_safeState[_safe].scheduledTransactions[txHash].executionTime != 0) { + revert TimelockGuard_TransactionAlreadyScheduled(); + } + + // Verify signatures using the Safe's signature checking logic + // This function call reverts if the signatures are invalid. + _safe.checkSignatures(txHash, txHashData, _signatures); + + // Calculate the execution time + uint256 executionTime = block.timestamp + _safeState[_safe].timelockDelay; + + // Schedule the transaction and add it to the pending transactions set + _safeState[_safe].scheduledTransactions[txHash] = + ScheduledTransaction({ executionTime: executionTime, state: TransactionState.Pending, params: _params }); + _safeState[_safe].pendingTxHashes.add(txHash); + + emit TransactionScheduled(_safe, txHash, executionTime); + } + + /// @notice Cancel a scheduled transaction if cancellation threshold is met + /// @dev This function aims to mimic the approach which would be used by a quorum of signers to + /// cancel a partially signed transaction, by signing and executing an empty + /// transaction at the same nonce. + /// This enables us to define a standard "cancellation transaction" format using the Safe address, nonce, + /// and hash of the transaction being cancelled. This is necessary to ensure that the cancellation transaction + /// is unique and cannot be used to cancel another transaction at the same nonce. + /// + /// Signature verification uses the Safe's checkNSignatures function, so that the number of signatures + /// can be set by the Safe's current cancellation threshold. Another benefit of checkNSignatures is that owners + /// can use any method to sign the cancellation transaction inputs, including signing with a private key, + /// calling the Safe's approveHash function, or EIP1271 contract signatures. + /// @param _safe The Safe address to cancel the transaction for. + /// @param _txHash The hash of the transaction being cancelled. + /// @param _nonce The nonce of the Safe for the transaction being cancelled. + /// @param _signatures The signatures of the owners who are cancelling the transaction. + function cancelTransaction(Safe _safe, bytes32 _txHash, uint256 _nonce, bytes memory _signatures) external { + // The following checks ensure that the transaction has: + // 1. Been scheduled + // 2. Not already been cancelled + // 3. Not already been executed + // There is nothing inherently wrong with cancelling a transaction a transaction that doesn't meet these + // criteria, but we revert in order to inform the user, and avoid emitting a misleading TransactionCancelled + // event. + if (_safeState[_safe].scheduledTransactions[_txHash].state == TransactionState.Cancelled) { + revert TimelockGuard_TransactionAlreadyCancelled(); + } + if (_safeState[_safe].scheduledTransactions[_txHash].state == TransactionState.Executed) { + revert TimelockGuard_TransactionAlreadyExecuted(); + } + if (_safeState[_safe].scheduledTransactions[_txHash].state == TransactionState.NotScheduled) { + revert TimelockGuard_TransactionNotScheduled(); + } + + // Generate the cancellation transaction data + bytes memory txData = abi.encodeCall(this.signCancellation, (_txHash)); + // Any nonce can be used here, as long as all of the signatures are for the same + // nonce. In practice we expect the nonce to be the same as the nonce of the transaction + // being cancelled, as this most closely mimics the behaviour of the Safe UI's transaction + // replacement feature. However we do not enforce that here, to allow for flexibility, + // and to avoid the need for logic to retrieve the nonce from the transaction being + // cancelled. + bytes memory cancellationTxData = _safe.encodeTransactionData( + address(this), 0, txData, Enum.Operation.Call, 0, 0, 0, address(0), address(0), _nonce + ); + bytes32 cancellationTxHash = _safe.getTransactionHash( + address(this), 0, txData, Enum.Operation.Call, 0, 0, 0, address(0), address(0), _nonce + ); + + // Verify signatures using the Safe's signature checking logic, with the cancellation threshold as + // the number of signatures required. + _safe.checkNSignatures( + cancellationTxHash, cancellationTxData, _signatures, _safeState[_safe].cancellationThreshold + ); + + // Set the transaction as cancelled, and remove it from the pending transactions set + _safeState[_safe].scheduledTransactions[_txHash].state = TransactionState.Cancelled; + _safeState[_safe].pendingTxHashes.remove(_txHash); + + // Increase the cancellation threshold + _increaseCancellationThreshold(_safe); + + emit TransactionCancelled(_safe, _txHash); + } + + //////////////////////////////////////////////////////////////// + // Dummy Functions // + //////////////////////////////////////////////////////////////// + + /// @notice Dummy function provided as a utility to facilitate signing cancelTransaction data in + /// the Safe UI. + function signCancellation(bytes32) public { + emit Message("This function is not meant to be called, did you mean to call cancelTransaction?"); + } +} diff --git a/packages/contracts-bedrock/test/safe/TimelockGuard.t.sol b/packages/contracts-bedrock/test/safe/TimelockGuard.t.sol new file mode 100644 index 0000000000000..88e12657997d4 --- /dev/null +++ b/packages/contracts-bedrock/test/safe/TimelockGuard.t.sol @@ -0,0 +1,879 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { Test } from "forge-std/Test.sol"; +import { GnosisSafe as Safe } from "safe-contracts/GnosisSafe.sol"; +import { GuardManager } from "safe-contracts/base/GuardManager.sol"; +import "test/safe-tools/SafeTestTools.sol"; + +import { TimelockGuard } from "src/safe/TimelockGuard.sol"; + +using TransactionBuilder for TransactionBuilder.Transaction; + +/// @title TransactionBuilder +/// @notice Facilitates the construction of transactions and signatures, and provides helper methods +/// for scheduling, executing, and cancelling transactions. +library TransactionBuilder { + // A struct type used to construct a transaction for scheduling and execution + struct Transaction { + SafeInstance safeInstance; + TimelockGuard.ExecTransactionParams params; + uint256 nonce; + bytes32 hash; + bytes signatures; + } + + address internal constant VM_ADDR = 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D; + + /// @notice Sets a nonce value on the provided transaction struct. + function setNonce(Transaction memory _tx, uint256 _nonce) internal pure { + _tx.nonce = _nonce; + } + + /// @notice Computes and stores the Safe transaction hash for the struct. + function setHash(Transaction memory _tx) internal view { + _tx.hash = _tx.safeInstance.safe.getTransactionHash({ + to: _tx.params.to, + value: _tx.params.value, + data: _tx.params.data, + operation: _tx.params.operation, + safeTxGas: _tx.params.safeTxGas, + baseGas: _tx.params.baseGas, + gasPrice: _tx.params.gasPrice, + gasToken: _tx.params.gasToken, + refundReceiver: _tx.params.refundReceiver, + _nonce: _tx.nonce + }); + } + + /// @notice Collects signatures from the first `_num` owners for the transaction. + function setSignatures(Transaction memory _tx, uint256 _num) internal pure { + bytes memory signatures = new bytes(0); + for (uint256 i; i < _num; ++i) { + (uint8 v, bytes32 r, bytes32 s) = Vm(VM_ADDR).sign(_tx.safeInstance.ownerPKs[i], _tx.hash); + + // The signature format is a compact form of: {bytes32 r}{bytes32 s}{uint8 v} + signatures = bytes.concat(signatures, abi.encodePacked(r, s, v)); + } + _tx.signatures = signatures; + } + + /// @notice Collects enough signatures to meet the Safe threshold. + function setSignatures(Transaction memory _tx) internal view { + uint256 num = _tx.safeInstance.safe.getThreshold(); + setSignatures(_tx, num); + } + + /// @notice Updates the hash and signatures for a specific approval count. + function updateTransaction(Transaction memory _tx, uint256 _num) internal view { + _tx.setHash(); + _tx.setSignatures(_num); + } + + /// @notice Updates the hash and threshold-based signatures on the transaction. + function updateTransaction(Transaction memory _tx) internal view { + _tx.setHash(); + _tx.setSignatures(); + } + + /// @notice Schedules the transaction with the supplied TimelockGuard instance. + function scheduleTransaction(Transaction memory _tx, TimelockGuard _timelockGuard) internal { + _timelockGuard.scheduleTransaction(_tx.safeInstance.safe, _tx.nonce, _tx.params, _tx.signatures); + } + + /// @notice Executes the transaction via the underlying Safe contract. + function executeTransaction(Transaction memory _tx) internal { + _tx.safeInstance.safe.execTransaction( + _tx.params.to, + _tx.params.value, + _tx.params.data, + _tx.params.operation, + _tx.params.safeTxGas, + _tx.params.baseGas, + _tx.params.gasPrice, + _tx.params.gasToken, + _tx.params.refundReceiver, + _tx.signatures + ); + } + + /// @notice Returns a fresh transaction struct copy with identical fields. + function deepCopy(Transaction memory _tx) internal pure returns (Transaction memory) { + return Transaction({ + safeInstance: _tx.safeInstance, + nonce: _tx.nonce, + params: _tx.params, + signatures: _tx.signatures, + hash: _tx.hash + }); + } + + /// @notice Builds the corresponding cancellation transaction for the provided data. + function makeCancellationTransaction( + Transaction memory _tx, + TimelockGuard _timelockGuard + ) + internal + view + returns (Transaction memory) + { + // Deep copy the transaction + Transaction memory cancellation = Transaction({ + safeInstance: _tx.safeInstance, + nonce: _tx.nonce, + params: _tx.params, + signatures: _tx.signatures, + hash: _tx.hash + }); + + // Empty out the params, then set based on the cancellation transaction format + delete cancellation.params; + cancellation.params.to = address(_timelockGuard); + cancellation.params.data = abi.encodeCall(TimelockGuard.signCancellation, (_tx.hash)); + + // Get only the number of signatures required for the cancellation transaction + uint256 cancellationThreshold = _timelockGuard.cancellationThreshold(_tx.safeInstance.safe); + + cancellation.updateTransaction(cancellationThreshold); + return cancellation; + } +} + +/// @title TimelockGuard_TestInit +/// @notice Reusable test initialization for `TimelockGuard` tests. +contract TimelockGuard_TestInit is Test, SafeTestTools { + // Events + event GuardConfigured(Safe indexed safe, uint256 timelockDelay); + event TransactionScheduled(Safe indexed safe, bytes32 indexed txId, uint256 when); + event TransactionCancelled(Safe indexed safe, bytes32 indexed txId); + event CancellationThresholdUpdated(Safe indexed safe, uint256 oldThreshold, uint256 newThreshold); + event TransactionExecuted(Safe indexed safe, bytes32 txHash); + event Message(string message); + + uint256 constant INIT_TIME = 10; + uint256 constant TIMELOCK_DELAY = 7 days; + uint256 constant NUM_OWNERS = 5; + uint256 constant THRESHOLD = 3; + uint256 constant ONE_YEAR = 365 days; + + TimelockGuard timelockGuard; + + // The Safe address will be the same as SafeInstance.safe, but it has the Safe type. + // This is useful for testing functions that take a Safe as an argument. + Safe safe; + SafeInstance safeInstance; + + SafeInstance unguardedSafe; + + /// @notice Deploys test fixtures and configures default Safe instances. + function setUp() public virtual { + vm.warp(INIT_TIME); + + // Deploy the singleton TimelockGuard + timelockGuard = new TimelockGuard(); + // Set up Safe with owners + safeInstance = _deploySafe("owners", NUM_OWNERS, THRESHOLD); + safe = Safe(payable(safeInstance.safe)); + + // Safe without guard enabled + unguardedSafe = _deploySafe("owners-unguarded", NUM_OWNERS, THRESHOLD); + + // Enable the guard on the Safe + _enableGuard(safeInstance); + } + + /// @notice Deploys a Safe with the given owners and threshold + function _deploySafe( + string memory _prefix, + uint256 _numOwners, + uint256 _threshold + ) + internal + returns (SafeInstance memory) + { + (, uint256[] memory keys) = SafeTestLib.makeAddrsAndKeys(_prefix, _numOwners); + return _setupSafe(keys, _threshold); + } + + /// @notice Builds an empty transaction wrapper for a Safe instance. + function _createEmptyTransaction(SafeInstance memory _safeInstance) + internal + view + returns (TransactionBuilder.Transaction memory) + { + TransactionBuilder.Transaction memory transaction; + // transaction.params will have null values + transaction.safeInstance = _safeInstance; + transaction.nonce = _safeInstance.safe.nonce(); + transaction.updateTransaction(); + return transaction; + } + + /// @notice Creates a dummy transaction populated with placeholder call data. + function _createDummyTransaction(SafeInstance memory _safeInstance) + internal + view + returns (TransactionBuilder.Transaction memory) + { + TransactionBuilder.Transaction memory transaction = _createEmptyTransaction(_safeInstance); + transaction.params.to = address(0xabba); + transaction.params.data = hex"acdc"; + transaction.updateTransaction(); + return transaction; + } + + /// @notice Helper to configure the TimelockGuard for a Safe + function _configureGuard(SafeInstance memory _safe, uint256 _delay) internal { + SafeTestLib.execTransaction( + _safe, address(timelockGuard), 0, abi.encodeCall(TimelockGuard.configureTimelockGuard, (_delay)) + ); + } + + /// @notice Helper to enable guard on a Safe + function _enableGuard(SafeInstance memory _safe) internal { + SafeTestLib.execTransaction( + _safe, address(_safe.safe), 0, abi.encodeCall(GuardManager.setGuard, (address(timelockGuard))) + ); + } +} + +/// @title TimelockGuard_TimelockConfiguration_Test +/// @notice Tests for timelockConfiguration function +contract TimelockGuard_TimelockConfiguration_Test is TimelockGuard_TestInit { + /// @notice Ensures an unconfigured Safe reports a zero timelock delay. + function test_timelockConfiguration_returnsZeroForUnconfiguredSafe_succeeds() external view { + uint256 delay = timelockGuard.timelockConfiguration(safeInstance.safe); + assertEq(delay, 0); + // configured is now determined by timelockDelay == 0 + assertEq(delay == 0, true); + } + + /// @notice Validates the configuration view reflects the stored timelock delay. + function test_timelockConfiguration_returnsConfigurationForConfiguredSafe_succeeds() external { + _configureGuard(safeInstance, TIMELOCK_DELAY); + uint256 delay = timelockGuard.timelockConfiguration(safeInstance.safe); + assertEq(delay, TIMELOCK_DELAY); + // configured is now determined by timelockDelay != 0 + assertEq(delay != 0, true); + } +} + +/// @title TimelockGuard_ConfigureTimelockGuard_Test +/// @notice Tests for configureTimelockGuard function +contract TimelockGuard_ConfigureTimelockGuard_Test is TimelockGuard_TestInit { + /// @notice Verifies the guard can be configured with a standard delay. + function test_configureTimelockGuard_succeeds() external { + vm.expectEmit(true, true, true, true); + emit GuardConfigured(safe, TIMELOCK_DELAY); + + _configureGuard(safeInstance, TIMELOCK_DELAY); + + uint256 delay = timelockGuard.timelockConfiguration(safe); + assertEq(delay, TIMELOCK_DELAY); + // configured is now determined by timelockDelay != 0 + assertEq(delay != 0, true); + } + + /// @notice Confirms delays above the maximum revert during configuration. + function test_configureTimelockGuard_revertsIfDelayTooLong_reverts() external { + uint256 tooLongDelay = ONE_YEAR + 1; + + vm.expectRevert(TimelockGuard.TimelockGuard_InvalidTimelockDelay.selector); + vm.prank(address(safeInstance.safe)); + timelockGuard.configureTimelockGuard(tooLongDelay); + } + + /// @notice Checks configuration reverts when the contract is too old. + function test_configureTimelockGuard_revertsIfVersionTooOld_reverts() external { + // nosemgrep: sol-style-use-abi-encodecall + vm.mockCall(address(safeInstance.safe), abi.encodeWithSignature("VERSION()"), abi.encode("1.2.0")); + vm.expectRevert(TimelockGuard.TimelockGuard_InvalidVersion.selector, address(timelockGuard)); + vm.prank(address(safeInstance.safe)); + timelockGuard.configureTimelockGuard(TIMELOCK_DELAY); + } + + /// @notice Asserts the maximum valid delay configures successfully. + function test_configureTimelockGuard_acceptsMaxValidDelay_succeeds() external { + vm.expectEmit(true, true, true, true); + emit GuardConfigured(safe, ONE_YEAR); + + _configureGuard(safeInstance, ONE_YEAR); + + uint256 delay = timelockGuard.timelockConfiguration(safe); + assertEq(delay, ONE_YEAR); + // configured is now determined by timelockDelay != 0 + assertEq(delay != 0, true); + } + + /// @notice Demonstrates the guard can be reconfigured to a new delay. + function test_configureTimelockGuard_allowsReconfiguration_succeeds() external { + // Initial configuration + _configureGuard(safeInstance, TIMELOCK_DELAY); + assertEq(timelockGuard.timelockConfiguration(safe), TIMELOCK_DELAY); + + uint256 newDelay = TIMELOCK_DELAY + 1; + + // Setup and schedule the reconfiguration transaction + TransactionBuilder.Transaction memory reconfigureGuardTx = _createEmptyTransaction(safeInstance); + reconfigureGuardTx.params.to = address(timelockGuard); + reconfigureGuardTx.params.data = abi.encodeCall(TimelockGuard.configureTimelockGuard, (newDelay)); + reconfigureGuardTx.updateTransaction(); + reconfigureGuardTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + + // Reconfigure with different delay + vm.expectEmit(true, true, true, true); + emit GuardConfigured(safe, newDelay); + + _configureGuard(safeInstance, newDelay); + assertEq(timelockGuard.timelockConfiguration(safe), newDelay); + } + + /// @notice Ensures setting delay to zero clears the configuration. + function test_configureTimelockGuard_clearConfiguration_succeeds() external { + // First configure the guard + _configureGuard(safeInstance, TIMELOCK_DELAY); + assertEq(timelockGuard.timelockConfiguration(safe), TIMELOCK_DELAY); + + // Configure timelock delay to 0 should succeed and emit event + vm.expectEmit(true, true, true, true); + emit GuardConfigured(safe, 0); + vm.prank(address(safeInstance.safe)); + timelockGuard.configureTimelockGuard(0); + + // Timelock delay should be set to 0 + assertEq(timelockGuard.timelockConfiguration(safe), 0); + } + + /// @notice Checks clearing succeeds even if the guard was never configured. + function test_configureTimelockGuard_notConfigured_succeeds() external { + // Try to clear - should succeed even if not yet configured + vm.expectEmit(true, true, true, true); + emit GuardConfigured(safe, 0); + vm.prank(address(safeInstance.safe)); + timelockGuard.configureTimelockGuard(0); + } +} + +/// @title TimelockGuard_CancellationThreshold_Test +/// @notice Tests for cancellationThreshold function +contract TimelockGuard_CancellationThreshold_Test is TimelockGuard_TestInit { + /// @notice Validates cancellation threshold is zero when the guard is disabled. + function test_cancellationThreshold_returnsZeroIfGuardNotEnabled_succeeds() external view { + uint256 threshold = timelockGuard.cancellationThreshold(Safe(payable(unguardedSafe.safe))); + assertEq(threshold, 0); + } + + /// @notice Ensures an enabled but unconfigured guard yields a zero threshold. + function test_cancellationThreshold_returnsZeroIfGuardNotConfigured_succeeds() external view { + // Safe with guard enabled but not configured should return 0 + uint256 threshold = timelockGuard.cancellationThreshold(safe); + assertEq(threshold, 0); + } + + /// @notice Confirms the default threshold becomes one after configuration. + function test_cancellationThreshold_returnsOneAfterConfiguration_succeeds() external { + // Configure the guard + _configureGuard(safeInstance, TIMELOCK_DELAY); + + // Should default to 1 after configuration + uint256 threshold = timelockGuard.cancellationThreshold(safe); + assertEq(threshold, 1); + } + + // 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 { + /// @notice Configures the guard before each scheduleTransaction test. + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + /// @notice Ensures scheduling emits the expected event and stores state. + function test_scheduleTransaction_succeeds() public { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + + vm.expectEmit(true, true, true, true); + emit TransactionScheduled(safe, dummyTx.hash, INIT_TIME + TIMELOCK_DELAY); + dummyTx.scheduleTransaction(timelockGuard); + } + + // A test which demonstrates that if the guard is enabled but not explicitly configured, + // the timelock delay is set to 0. + /// @notice Checks scheduling reverts if the guard lacks configuration. + function test_scheduleTransaction_guardNotConfigured_reverts() external { + // Enable the guard on the unguarded Safe, but don't configure it + _enableGuard(unguardedSafe); + assertEq(timelockGuard.timelockConfiguration(unguardedSafe.safe), 0); + + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(unguardedSafe); + vm.expectRevert(TimelockGuard.TimelockGuard_GuardNotConfigured.selector); + dummyTx.scheduleTransaction(timelockGuard); + } + + /// @notice Verifies rescheduling an identical pending transaction reverts. + function test_scheduleTransaction_reschedulingIdenticalTransaction_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + + timelockGuard.scheduleTransaction(safeInstance.safe, dummyTx.nonce, dummyTx.params, dummyTx.signatures); + + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionAlreadyScheduled.selector); + timelockGuard.scheduleTransaction(dummyTx.safeInstance.safe, dummyTx.nonce, dummyTx.params, dummyTx.signatures); + } + + /// @notice Confirms scheduling fails when the guard has not been enabled. + function test_scheduleTransaction_guardNotEnabled_reverts() external { + // Attempt to schedule a transaction with a Safe that has enabled the guard but + // has not configured it. + _enableGuard(unguardedSafe); + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(unguardedSafe); + + vm.expectRevert(TimelockGuard.TimelockGuard_GuardNotConfigured.selector); + dummyTx.scheduleTransaction(timelockGuard); + } + + /// @notice Demonstrates identical payloads can be scheduled with distinct nonces. + function test_scheduleTransaction_canScheduleIdenticalWithDifferentNonce_succeeds() external { + // Schedule a transaction with a specific nonce + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Schedule an identical transaction with a different nonce (salt) + TransactionBuilder.Transaction memory newTx = dummyTx.deepCopy(); + newTx.nonce = dummyTx.nonce + 1; + newTx.updateTransaction(); + + vm.expectEmit(true, true, true, true); + emit TransactionScheduled(safe, newTx.hash, INIT_TIME + TIMELOCK_DELAY); + timelockGuard.scheduleTransaction(safeInstance.safe, newTx.nonce, newTx.params, newTx.signatures); + } +} + +/// @title TimelockGuard_ScheduledTransaction_Test +/// @notice Tests for scheduledTransaction function +contract TimelockGuard_ScheduledTransaction_Test is TimelockGuard_TestInit { + /// @notice Configures the guard before each scheduleTransaction test. + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + function test_scheduledTransaction_succeeds() external { + // schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + TimelockGuard.ScheduledTransaction memory scheduledTransaction = + timelockGuard.scheduledTransaction(safe, dummyTx.hash); + assertEq(scheduledTransaction.executionTime, INIT_TIME + TIMELOCK_DELAY); + assert(scheduledTransaction.state == TimelockGuard.TransactionState.Pending); + assertEq(keccak256(abi.encode(scheduledTransaction.params)), keccak256(abi.encode(dummyTx.params))); + } +} + +/// @title TimelockGuard_PendingTransactions_Test +/// @notice Tests for pendingTransactions function +contract TimelockGuard_PendingTransactions_Test is TimelockGuard_TestInit { + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + function test_pendingTransactions_succeeds() external { + // schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + TimelockGuard.ScheduledTransaction[] memory pendingTransactions = timelockGuard.pendingTransactions(safe); + assertEq(pendingTransactions.length, 1); + // ensure the hash of the transaction params are the same + assertEq(pendingTransactions[0].params.to, dummyTx.params.to); + assertEq(keccak256(abi.encode(pendingTransactions[0].params)), keccak256(abi.encode(dummyTx.params))); + } + + function test_pendingTransactions_removeTransactionAfterCancellation_succeeds() external { + // schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // cancel the transaction + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + + // get the pending transactions + TimelockGuard.ScheduledTransaction[] memory pendingTransactions = timelockGuard.pendingTransactions(safe); + assertEq(pendingTransactions.length, 0); + } + + function test_pendingTransactions_removeTransactionAfterExecution_succeeds() external { + // schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + + // execute the transaction + dummyTx.executeTransaction(); + + // get the pending transactions + TimelockGuard.ScheduledTransaction[] memory pendingTransactions = timelockGuard.pendingTransactions(safe); + assertEq(pendingTransactions.length, 0); + } +} + +/// @title TimelockGuard_signCancellation_Test +/// @notice Tests for signCancellation function +contract TimelockGuard_signCancellation_Test is TimelockGuard_TestInit { + function test_signCancellation_succeeds() external { + vm.expectEmit(true, true, true, true); + emit Message("This function is not meant to be called, did you mean to call cancelTransaction?"); + timelockGuard.signCancellation(bytes32(0)); + } +} + +contract TimelockGuard_CancelTransaction_Test is TimelockGuard_TestInit { + /// @notice Prepares a configured guard before cancellation tests run. + function setUp() public override { + super.setUp(); + + // Configure the guard and schedule a transaction + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + /// @notice Ensures cancellations succeed using owner signatures. + function test_cancelTransaction_withPrivKeySignature_succeeds() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Get the cancellation transaction + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + uint256 cancellationThreshold = timelockGuard.cancellationThreshold(dummyTx.safeInstance.safe); + + // Cancel the transaction + vm.expectEmit(true, true, true, true); + emit CancellationThresholdUpdated(safeInstance.safe, cancellationThreshold, cancellationThreshold + 1); + vm.expectEmit(true, true, true, true); + emit TransactionCancelled(safeInstance.safe, dummyTx.hash); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + + assert( + timelockGuard.scheduledTransaction(safeInstance.safe, dummyTx.hash).state + == TimelockGuard.TransactionState.Cancelled + ); + } + + /// @notice Confirms pre-approved hashes can authorise cancellations. + function test_cancelTransaction_withApproveHash_succeeds() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Get the cancellation transaction hash + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + + // Get the owner + address owner = dummyTx.safeInstance.safe.getOwners()[0]; + + // Approve the cancellation transaction hash + vm.prank(owner); + safeInstance.safe.approveHash(cancellationTx.hash); + + // Encode the prevalidated cancellation signature + bytes memory cancellationSignatures = abi.encodePacked(bytes32(uint256(uint160(owner))), bytes32(0), uint8(1)); + + // Get the cancellation threshold + uint256 cancellationThreshold = timelockGuard.cancellationThreshold(dummyTx.safeInstance.safe); + + // Cancel the transaction + vm.expectEmit(true, true, true, true); + emit CancellationThresholdUpdated(dummyTx.safeInstance.safe, cancellationThreshold, cancellationThreshold + 1); + vm.expectEmit(true, true, true, true); + emit TransactionCancelled(dummyTx.safeInstance.safe, dummyTx.hash); + timelockGuard.cancelTransaction(dummyTx.safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationSignatures); + + // Confirm that the transaction is cancelled + TimelockGuard.ScheduledTransaction memory scheduledTransaction = + timelockGuard.scheduledTransaction(dummyTx.safeInstance.safe, dummyTx.hash); + assert(scheduledTransaction.state == TimelockGuard.TransactionState.Cancelled); + } + + /// @notice Verifies cancelling an unscheduled transaction reverts. + function test_cancelTransaction_revertsIfTransactionNotScheduled_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + + // Attempt to cancel the transaction + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionNotScheduled.selector); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + } +} + +/// @title TimelockGuard_CheckTransaction_Test +/// @notice Tests for checkTransaction function +contract TimelockGuard_CheckTransaction_Test is TimelockGuard_TestInit { + /// @notice Establishes the configured guard before checkTransaction tests. + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + /// @notice Test that checkTransaction reverts when scheduled transaction delay hasn't passed + function test_checkTransaction_scheduledTransactionNotReady_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + + // Schedule the transaction but do not advance time past the timelock delay + dummyTx.scheduleTransaction(timelockGuard); + + // Increment the nonce, as would normally happen when the transaction is executed + vm.store(address(safeInstance.safe), bytes32(uint256(5)), bytes32(uint256(safeInstance.safe.nonce() + 1))); + + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionNotReady.selector); + vm.prank(address(safeInstance.safe)); + timelockGuard.checkTransaction( + dummyTx.params.to, + dummyTx.params.value, + dummyTx.params.data, + dummyTx.params.operation, + dummyTx.params.safeTxGas, + dummyTx.params.baseGas, + dummyTx.params.gasPrice, + dummyTx.params.gasToken, + dummyTx.params.refundReceiver, + "", + address(0) + ); + } + + /// @notice Test that checkTransaction reverts when scheduled transaction was cancelled + function test_checkTransaction_scheduledTransactionCancelled_reverts() external { + // Schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Cancel the transaction + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + + // Fast forward past the timelock delay + vm.warp(block.timestamp + TIMELOCK_DELAY); + // Increment the nonce, as would normally happen when the transaction is executed + vm.store(address(safeInstance.safe), bytes32(uint256(5)), bytes32(uint256(safeInstance.safe.nonce() + 1))); + + // Should revert because transaction was cancelled + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionAlreadyCancelled.selector); + vm.prank(address(safeInstance.safe)); + timelockGuard.checkTransaction( + dummyTx.params.to, + dummyTx.params.value, + dummyTx.params.data, + dummyTx.params.operation, + dummyTx.params.safeTxGas, + dummyTx.params.baseGas, + dummyTx.params.gasPrice, + dummyTx.params.gasToken, + dummyTx.params.refundReceiver, + "", + address(0) + ); + } + + /// @notice Test that checkTransaction reverts when a transaction has not been scheduled + function test_checkTransaction_transactionNotScheduled_reverts() external { + // Get transaction parameters but don't schedule the transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + + // Should revert because transaction was not scheduled + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionNotScheduled.selector); + vm.prank(address(safeInstance.safe)); + timelockGuard.checkTransaction( + dummyTx.params.to, + dummyTx.params.value, + dummyTx.params.data, + dummyTx.params.operation, + dummyTx.params.safeTxGas, + dummyTx.params.baseGas, + dummyTx.params.gasPrice, + dummyTx.params.gasToken, + dummyTx.params.refundReceiver, + "", + address(0) + ); + } +} + +/// @title TimelockGuard_MaxCancellationThreshold_Test +/// @notice Tests for the maxCancellationThreshold function in TimelockGuard +contract TimelockGuard_MaxCancellationThreshold_Test is TimelockGuard_TestInit { + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + /// @notice Test that maxCancellationThreshold returns the correct value + function test_maxCancellationThreshold_maxThresholdIsBlockingThreshold_succeeds() external { + // create a new Safe with 7 owners and quorum of 5 (blocking threshold is 3) + SafeInstance memory newSafeInstance = _deploySafe("owners", 7, 5); + _enableGuard(newSafeInstance); + _configureGuard(newSafeInstance, TIMELOCK_DELAY); + + // Set up a dummy transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(newSafeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Calculate expected max cancellation threshold + uint256 blockingThreshold = newSafeInstance.safe.getOwners().length - newSafeInstance.safe.getThreshold() + 1; + uint256 quorum = newSafeInstance.safe.getThreshold(); + + // Ensure that the minimum is set by the blocking threshold + assertGt(quorum, blockingThreshold); + + // Assert that the maxCancellationThreshold function returns the expected value + assertEq(timelockGuard.maxCancellationThreshold(newSafeInstance.safe), blockingThreshold); + } + + /// @notice Test that maxCancellationThreshold returns the correct value + function test_maxCancellationThreshold_maxThresholdIsQuorum_succeeds() external { + // create a new Safe with 7 owners and quorum of 3 (blocking threshold is 5) + SafeInstance memory newSafeInstance = _deploySafe("owners", 7, 3); + _enableGuard(newSafeInstance); + _configureGuard(newSafeInstance, TIMELOCK_DELAY); + + // Set up a dummy transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(newSafeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + // Calculate expected max cancellation threshold + uint256 blockingThreshold = newSafeInstance.safe.getOwners().length - newSafeInstance.safe.getThreshold() + 1; + uint256 quorum = newSafeInstance.safe.getThreshold(); + + // Ensure that the minimum is set by quorum + assertGt(blockingThreshold, quorum); + + // Assert that the maxCancellationThreshold function returns the expected value + assertEq(timelockGuard.maxCancellationThreshold(newSafeInstance.safe), quorum); + } +} + +/// @title TimelockGuard_Integration_Test +/// @notice Tests for integration between TimelockGuard and Safe +contract TimelockGuard_Integration_Test is TimelockGuard_TestInit { + using stdStorage for StdStorage; + + function setUp() public override { + super.setUp(); + _configureGuard(safeInstance, TIMELOCK_DELAY); + } + + /// @notice Test that scheduling a transaction and then executing it succeeds + function test_integration_scheduleThenExecute_succeeds() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + + // increment the cancellation threshold so that we can test that it is reset + uint256 slot = stdstore.target(address(timelockGuard)).sig("cancellationThreshold(address)").with_key( + address(safeInstance.safe) + ).find(); + vm.store( + address(timelockGuard), + bytes32(slot), + bytes32(uint256(timelockGuard.cancellationThreshold(safeInstance.safe) + 1)) + ); + + vm.expectEmit(true, true, true, true); + emit TransactionExecuted(safeInstance.safe, dummyTx.hash); + dummyTx.executeTransaction(); + + // Confirm that the transaction is executed + TimelockGuard.ScheduledTransaction memory scheduledTransaction = + timelockGuard.scheduledTransaction(safeInstance.safe, dummyTx.hash); + assert(scheduledTransaction.state == TimelockGuard.TransactionState.Executed); + + // Confirm that the cancellation threshold is reset + assertEq(timelockGuard.cancellationThreshold(safeInstance.safe), 1); + } + + /// @notice Test that scheduling a transaction and then executing it twice reverts + function test_integration_scheduleThenExecuteTwice_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + dummyTx.executeTransaction(); + + vm.expectRevert("GS026"); + dummyTx.executeTransaction(); + } + + function test_integration_scheduleThenExecuteThenCancel_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + dummyTx.executeTransaction(); + + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionAlreadyExecuted.selector); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + } + + /// @notice Test that rescheduling an identical previously cancelled transaction reverts + function test_integration_scheduleTransactionIdenticalToPreviouslyCancelled_reverts() external { + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + dummyTx.scheduleTransaction(timelockGuard); + + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + + vm.expectRevert(TimelockGuard.TimelockGuard_TransactionAlreadyScheduled.selector); + dummyTx.scheduleTransaction(timelockGuard); + } + + /// @notice Test that the guard can be reset while still enabled, and then can be disabled + function test_integration_resetThenDisableGuard_succeeds() external { + TransactionBuilder.Transaction memory resetGuardTx = _createEmptyTransaction(safeInstance); + resetGuardTx.params.to = address(timelockGuard); + resetGuardTx.params.data = abi.encodeCall(TimelockGuard.configureTimelockGuard, (0)); + resetGuardTx.updateTransaction(); + resetGuardTx.scheduleTransaction(timelockGuard); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + resetGuardTx.executeTransaction(); + + TransactionBuilder.Transaction memory disableGuardTx = _createEmptyTransaction(safeInstance); + disableGuardTx.params.to = address(safeInstance.safe); + disableGuardTx.params.data = abi.encodeCall(GuardManager.setGuard, (address(0))); + disableGuardTx.updateTransaction(); + + vm.warp(block.timestamp + TIMELOCK_DELAY); + disableGuardTx.executeTransaction(); + } + + /// @notice Test that the max cancellation threshold is not exceeded + function test_integration_maxCancellationThresholdNotExceeded_succeeds() external { + uint256 maxThreshold = timelockGuard.maxCancellationThreshold(safeInstance.safe); + + // Schedule a transaction + TransactionBuilder.Transaction memory dummyTx = _createDummyTransaction(safeInstance); + + // schedule and cancel the transaction maxThreshold + 1 times + for (uint256 i = 0; i < maxThreshold + 1; i++) { + // modify the calldata slightly to make the txHash different + dummyTx.params.data = bytes.concat(dummyTx.params.data, abi.encodePacked(i)); + dummyTx.updateTransaction(); + dummyTx.scheduleTransaction(timelockGuard); + + // Cancel the transaction + TransactionBuilder.Transaction memory cancellationTx = dummyTx.makeCancellationTransaction(timelockGuard); + timelockGuard.cancelTransaction(safeInstance.safe, dummyTx.hash, dummyTx.nonce, cancellationTx.signatures); + } + + assertEq(timelockGuard.cancellationThreshold(safeInstance.safe), maxThreshold); + } +}