From c665d08bad9f43b2dffca33e9685484ffe67284c Mon Sep 17 00:00:00 2001 From: alcueca Date: Tue, 26 Aug 2025 07:50:16 +0200 Subject: [PATCH 1/7] initial draft --- specs/governance/timelock-guard.md | 136 +++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 specs/governance/timelock-guard.md diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md new file mode 100644 index 000000000..c62f61cf8 --- /dev/null +++ b/specs/governance/timelock-guard.md @@ -0,0 +1,136 @@ +# Governor + + + +**Table of Contents** + +- [Overview](#overview) +- [Definitions](#definitions) + - [`quorum(safe)`](#quorumsafe) + - [Scheduled Transaction](#scheduled-transaction) + - [`scheduling_time(safe, tx)`](#scheduling_timesafe-tx) + - [`delay_period(safe)`](#delay_periodsafe) + - [Rejected Transaction](#rejected-transaction) + - [`rejecting_owners(safe, tx)`](#rejecting_ownerssafe-tx) + - [`cancellation_threshold(safe)`](#cancellation_thresholdsafe) + - [Nested Safe Setup](#nested-safe-setup) + - [Nested Cancellation](#nested-cancellation) +- [Assumptions](#assumptions) +- [Invariants](#invariants) +- [Function Specification](#function-specification) + - [constructor](#constructor) + - [enable(delay_period)](#enabledelay_period) + - [disable](#disable) + - [cancellationThreshold(safe)](#cancellationthresholdsafe) + - [delayPeriod(safe)](#delayperiodsafe) + - [setDelayPeriod(delay_period)](#setdelayperioddelay_period) + - [scheduleTransaction](#scheduletransaction) + - [checkTransaction](#checktransaction) + - [rejectTransaction](#rejecttransaction) + - [cancelTransaction](#canceltransaction) + + + +## Overview + +The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a Safe Guard. Safe Guards are external contracts that Safes call into before and after the execution of a transaction to enquire if the execution fulfils conditions implemented in the Guard, and revert if they don't. The `TimelockGuard` + +## Definitions + +### `quorum(safe)` +The number of owners in a safe that must signal aproval of a transaction for it to be executed by the safe. + +### Scheduled Transaction +A specific transaction that has been stored in the Timelock, for execution from a given safe. + +### `scheduling_time(safe, tx)` +The time in seconds of the block in which a transaction was scheduled. + +### `delay_period(safe)` +Given a scheduled transaction, the minimum time in seconds after `scheduling_time` that execution is allowed. + +### Rejected Transaction +Given a transaction scheduled to execute from given safe, owners for the safe reject the transaction to signal they think the transaction should not be executed after the `delay_period`. + +### `rejecting_owners(safe, tx)` +Given a transaction scheduled to execute from given safe, the owners for the safe that rejected the transaction. + +### `cancellation_threshold(safe)` +The number of owners that must reject a given transaction for it not to be executed after the `delay_period`. It is equal to `blocking_minority`, where `blocking_minority = min(quorum(safe), total_owners(safe) - quorum + 1)`. + +Alternatively, the `cancellation_threshold` can start at 1 for each safe, increasing by 1 with each consecutive cancelled by the given safe transaction up to `blocking_minority`, resetting to 1 with each successfully executed transaction by the given safe. + +### Nested Safe Setup +A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. + +### Nested Cancellation +For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold` of a child safe must be considered before registering that the child safe is rejecting the transaction. + +## Assumptions + +## Invariants + +## Function Specification + +### constructor + +### enable(delay_period) +Called by a Safe, enable the TimelockGuard for it. + +Takes the `delay_period` as a parameter. + +- MUST revert if the TimelockGuard is already enabled for the safe. + +### disable +Called by a Safe, disable the TimelockGuard for it. + +- MUST revert if the TimelockGuard is already disabled for the safe. + +### cancellationThreshold(safe) +Returns the `cancellation_threshold` for a given safe. + +- MUST return 0 if the TimelockGuard is not enabled for the safe. + +### delayPeriod(safe) +Returns the `delay_period` for a given safe. + +- MUST return 0 if the TimelockGuard is not enabled for the safe. + +### setDelayPeriod(delay_period) +Called by a Safe, set its `delay_period`. + +- MUST revert if the TimelockGuard is not enabled for the safe. + +### scheduleTransaction +Called by anyone using signatures from Safe owners, registers a transaction in the TimelockGuard for execution after the `delay_period`. + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST take the same parameters as `execTransaction`. +- MUST revert if an identical transaction has already been scheduled. +- MUST revert if an identical transaction was cancelled. + +To allow for identical transactions to be scheduled more than once, but requiring different signatures for each one, a `salt` parameter can be included in `data` with the sole purpose of differentiating otherwise identical transactions. + +### checkTransaction +Called by anyone, and also by the Safe in `execTransaction`, verifies if the transaction was scheduled and the delay period has passed. + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST take the exact parameters from the Safe API +- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) < block.timestamp` +- MUST revert if the scheduled transaction was cancelled + +### rejectTransaction +Called by a Safe owner, signal the rejection of a scheduled transaction. +TODO: Should we use signatures here? + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST revert if not called by an owner of the safe. +- MUST be able to reference a transaction scheduled in a different safe. The transaction might not exist. + +### cancelTransaction +Called by anyone, verify that the `cancellation_threshold` has been met for the Safe to cancel a given scheduled transaction. + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) >= block.timestamp` +- MUST revert if `sum(rejecting_owners(safe, tx)) < cancellation_threshold(safe)` + From 5a0d620f9e71ed7e8744cdf604e94582a1975724 Mon Sep 17 00:00:00 2001 From: alcueca Date: Wed, 27 Aug 2025 08:55:02 +0200 Subject: [PATCH 2/7] Multiple comments addressed --- specs/governance/timelock-guard.md | 107 ++++++++++++++++++----------- 1 file changed, 67 insertions(+), 40 deletions(-) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index c62f61cf8..f7c1d2bc7 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -33,74 +33,82 @@ ## Overview -The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a Safe Guard. Safe Guards are external contracts that Safes call into before and after the execution of a transaction to enquire if the execution fulfils conditions implemented in the Guard, and revert if they don't. The `TimelockGuard` +The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a Safe Guard. Safe Guards are external contracts that Safes call into before and after the execution of a transaction to enquire if the execution fulfils conditions implemented in the Guard, and revert if they don't. The `TimelockGuard` is a Safe Guard that implements a mandatory delay on the execution of transactions. ## Definitions +The following list defines variables that will be used in TimelockGuard. None of the variable names are compulsory, and if better names are found, they should be used. -### `quorum(safe)` -The number of owners in a safe that must signal aproval of a transaction for it to be executed by the safe. +### `Quorum` +The `quorum(safe)` is the number of owners in a safe that must signal approval of a transaction for it to be executed by the safe. + +### Blocking Threshold +The `blocking_threshold(safe)` is the minimum number of owners that need to not signal approval so that a `quorum(safe)` can't be reached for the same safe. This is defined as `blocking_threshold(safe) = min(quorum(safe), total_owners(safe) - quorum(safe) + 1)`. ### Scheduled Transaction A specific transaction that has been stored in the Timelock, for execution from a given safe. -### `scheduling_time(safe, tx)` -The time in seconds of the block in which a transaction was scheduled. +### Scheduling Time +The `scheduling_time(safe, tx)` is the time in seconds of the block in which a transaction was scheduled for execution from a given safe. -### `delay_period(safe)` -Given a scheduled transaction, the minimum time in seconds after `scheduling_time` that execution is allowed. +### Delay Period +Given a scheduled transaction, the `delay_period(safe)` is the minimum time in seconds after `scheduling_time(safe, tx)` that execution is allowed. ### Rejected Transaction -Given a transaction scheduled to execute from given safe, owners for the safe reject the transaction to signal they think the transaction should not be executed after the `delay_period`. +Given a transaction scheduled to execute from given safe, owners for the safe reject the transaction to signal they think the transaction should not be executed after the `delay_period(safe)`. -### `rejecting_owners(safe, tx)` -Given a transaction scheduled to execute from given safe, the owners for the safe that rejected the transaction. +### Rejecting Owners +Given a transaction scheduled to execute from given safe, the `rejecting_owners(safe, tx)` are the owners for the safe that rejected the transaction. -### `cancellation_threshold(safe)` -The number of owners that must reject a given transaction for it not to be executed after the `delay_period`. It is equal to `blocking_minority`, where `blocking_minority = min(quorum(safe), total_owners(safe) - quorum + 1)`. +### Cancellation Threshold +The `cancellation_threshold(safe)` is the number of owners that must reject a given transaction for it not to be executed after the `delay_period(safe)`. It is equal to `blocking_threshold(safe)`. -Alternatively, the `cancellation_threshold` can start at 1 for each safe, increasing by 1 with each consecutive cancelled by the given safe transaction up to `blocking_minority`, resetting to 1 with each successfully executed transaction by the given safe. +Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, increasing by 1 with each consecutive cancelled by the given safe transaction up to `blocking_threshold(safe)`, resetting to 1 with each successfully executed transaction by the given safe. -### Nested Safe Setup -A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. +## Assumptions + +## Invariants ### Nested Cancellation -For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold` of a child safe must be considered before registering that the child safe is rejecting the transaction. +A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. -## Assumptions +For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be considered before registering that the child safe is rejecting the transaction. -## Invariants +To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe scheduled to execute the transaction, or in one of its child safes. -## Function Specification +### Module Execution +Safes implement separate Guard and ModuleGuard interfacesm and a Safe enables a separate Guard and ModuleGuard. To make the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: +- MUST implement both the Guard and ModuleGuard interfaces. +- `ITransactionGuard(timelockGuard).checkTransaction(...)` and `IModuleGuard(timelockGuard).checkModuleTransaction(...)` MUST execute the same delay checking logic. -### constructor +It follows that any module will need to implement an entry point function to `scheduleTransaction`. -### enable(delay_period) -Called by a Safe, enable the TimelockGuard for it. +### Replayability Conflicts +The TimelockGuard does not control replayability of transactions, which is done by either the safe or a potential module would decide to overwrite the logic. -Takes the `delay_period` as a parameter. +## Function Specification +The following list details the functions that must be included the in TimelockGuard. None of the function names are compulsory, and if better names are found, they should be used. + +### enable +Called by a Safe, enable the TimelockGuard for it and set the `delay_period`. -- MUST revert if the TimelockGuard is already enabled for the safe. +- MUST emit a `ModuleEnabled` event with at least `delay_period` as a parameter. ### disable Called by a Safe, disable the TimelockGuard for it. - MUST revert if the TimelockGuard is already disabled for the safe. +- MUST emit a `ModuleDisabled` event. -### cancellationThreshold(safe) +### cancellationThreshold Returns the `cancellation_threshold` for a given safe. - MUST return 0 if the TimelockGuard is not enabled for the safe. -### delayPeriod(safe) +### delayPeriod Returns the `delay_period` for a given safe. - MUST return 0 if the TimelockGuard is not enabled for the safe. -### setDelayPeriod(delay_period) -Called by a Safe, set its `delay_period`. - -- MUST revert if the TimelockGuard is not enabled for the safe. - ### scheduleTransaction Called by anyone using signatures from Safe owners, registers a transaction in the TimelockGuard for execution after the `delay_period`. @@ -108,29 +116,48 @@ Called by anyone using signatures from Safe owners, registers a transaction in t - MUST take the same parameters as `execTransaction`. - MUST revert if an identical transaction has already been scheduled. - MUST revert if an identical transaction was cancelled. +- MUST emit a `TransactionScheduled` event, with at least `safe` and relevant transaction data. To allow for identical transactions to be scheduled more than once, but requiring different signatures for each one, a `salt` parameter can be included in `data` with the sole purpose of differentiating otherwise identical transactions. ### checkTransaction -Called by anyone, and also by the Safe in `execTransaction`, verifies if the transaction was scheduled and the delay period has passed. +Called by anyone, and also by the Safe in `execTransaction`, verifies if a given transaction was scheduled and the delay period has passed. + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST take the exact parameters from the `ITransactionGuard.checkTransaction` interface. +- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) < block.timestamp`. +- MUST revert if the scheduled transaction was cancelled. + +### checkModuleTransaction +Called by anyone, and also by the Safe in `preModuleExecution`, verifies if a given transaction was scheduled and the delay period has passed. - MUST revert if the TimelockGuard is not enabled for the safe. -- MUST take the exact parameters from the Safe API -- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) < block.timestamp` -- MUST revert if the scheduled transaction was cancelled +- MUST take the exact parameters from the `IModuleGuard.checkModuleTransaction` interface. +- MUST execute the same internal logic as `checkTransaction`. + +### checkPendingTransactions +Called by anyone, returns the list of all scheduled but not cancelled transactions for a given safe. + +*Note:* If we want to exclude executed transactions from this list, the TimelockGuard would need to query into the storage of the safe, and if a module that overrides replayability is implemented, the TimelockGuard would need to look into its storage as well. ### rejectTransaction Called by a Safe owner, signal the rejection of a scheduled transaction. -TODO: Should we use signatures here? - MUST revert if the TimelockGuard is not enabled for the safe. -- MUST revert if not called by an owner of the safe. +- MUST revert if not called by an owner of the safe scheduled to execute the transaction, or in one of its child safes. - MUST be able to reference a transaction scheduled in a different safe. The transaction might not exist. +- MUST emit a `TransactionRejected` event, with at least `safe` and a transaction identifier. + +### rejectTransactionWithSignature +Called by anyone, using signatures form one or more owners, signal the rejection of a scheduled transaction. Can reuse the `rejectTransaction` function name. + +- MUST revert if the TimelockGuard is not enabled for the safe. +- MUST reuse the same internal logic as `rejectTransaction`. ### cancelTransaction Called by anyone, verify that the `cancellation_threshold` has been met for the Safe to cancel a given scheduled transaction. - MUST revert if the TimelockGuard is not enabled for the safe. -- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) >= block.timestamp` -- MUST revert if `sum(rejecting_owners(safe, tx)) < cancellation_threshold(safe)` - +- MUST revert if `scheduling_time(safe, tx) + delay_period(safe) >= block.timestamp`. +- MUST revert if `sum(rejecting_owners(safe, tx)) < cancellation_threshold(safe)`. +- MUST emit a `TransactionCancelled` event, with at least `safe` and a transaction identifier. From efd9a7bc387f3384db9bb65a76af634e3adbf583 Mon Sep 17 00:00:00 2001 From: alcueca Date: Wed, 27 Aug 2025 10:24:18 +0200 Subject: [PATCH 3/7] Formatted invariant --- specs/governance/timelock-guard.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index f7c1d2bc7..f9ba5ff17 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -68,6 +68,12 @@ Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, ## Invariants +### iTG-001: Honest Users Can Recover From Temporary Key Control Over a Quorum of Keys +If an attacker has joint or temporary key control over a quorum of keys, honest users should always be able to recover the account. + +#### Severity: Critical +If this invariant is broken, honest control of the multisig is lost. + ### Nested Cancellation A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. From 38e1e87e31f999f501440034ee7cb938134a5f49 Mon Sep 17 00:00:00 2001 From: alcueca Date: Thu, 28 Aug 2025 07:58:17 +0200 Subject: [PATCH 4/7] Extra assumption --- specs/governance/timelock-guard.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index f9ba5ff17..2cc786ce4 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -66,6 +66,9 @@ Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, ## Assumptions +### aTG-001: Dishonest Users Don't Have Permanent Key Control Over a Quorum of Keys +We assume that dishonest users have at most a temporary joint key control over a quorum of keys. + ## Invariants ### iTG-001: Honest Users Can Recover From Temporary Key Control Over a Quorum of Keys @@ -74,7 +77,7 @@ If an attacker has joint or temporary key control over a quorum of keys, honest #### Severity: Critical If this invariant is broken, honest control of the multisig is lost. -### Nested Cancellation +### iTG-002: Nested Cancellation A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be considered before registering that the child safe is rejecting the transaction. From 522cbc79ae14f5357ca0b4ce036e7a83edbea466 Mon Sep 17 00:00:00 2001 From: alcueca Date: Thu, 28 Aug 2025 08:04:08 +0200 Subject: [PATCH 5/7] More definitions, Requirements and Constraints section --- specs/governance/timelock-guard.md | 41 +++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index 2cc786ce4..1fb0e186e 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -38,11 +38,35 @@ The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a S ## Definitions The following list defines variables that will be used in TimelockGuard. None of the variable names are compulsory, and if better names are found, they should be used. -### `Quorum` -The `quorum(safe)` is the number of owners in a safe that must signal approval of a transaction for it to be executed by the safe. +### Quorum +The number of owners required to execute a transaction. ### Blocking Threshold -The `blocking_threshold(safe)` is the minimum number of owners that need to not signal approval so that a `quorum(safe)` can't be reached for the same safe. This is defined as `blocking_threshold(safe) = min(quorum(safe), total_owners(safe) - quorum(safe) + 1)`. +The minimum number of owners not willing to execute a transaction, that by not approving the transaction guarantee that `quorum` is not met. It is defined as `min(quorum, total_owners - quorum + 1)`. + +### Active Owner +An owner that is able to approve transactions. + +### Honest Owner +An owner that is never willing to execute transactions outside of governance processes. + +### Malicious Owner +An owner that is willing to execute transactions outside of governance processes. + +### Full Key Control +An actor has full key control of an owner if it is solely able to approve transactions as that owner. + +### Joint Key Control +An actor has joint key control of an owner if it is able, along with other actors, to approve transactions as that owner. + +### Temporary Key Control +An actor has temporary key control of an owner if it is able to approve a limited set or number of transactions as that owner. + +### Multisig Liveness Failure +A multisig is considered to be in a liveness failure state if the number of active honest owners is less than the `quorum`, or if the number of malicious active owners is equal or greater than the `blocking_threshold`. + +### Multisig Safety Failure +A multisig is considered to be in a safety failure state if the number of active malicious owners is equal or greater than `quorum`. ### Scheduled Transaction A specific transaction that has been stored in the Timelock, for execution from a given safe. @@ -69,6 +93,9 @@ Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, ### aTG-001: Dishonest Users Don't Have Permanent Key Control Over a Quorum of Keys We assume that dishonest users have at most a temporary joint key control over a quorum of keys. +#### Severity: Critical +If this invariant is broken, honest control of the multisig is lost. + ## Invariants ### iTG-001: Honest Users Can Recover From Temporary Key Control Over a Quorum of Keys @@ -77,7 +104,9 @@ If an attacker has joint or temporary key control over a quorum of keys, honest #### Severity: Critical If this invariant is broken, honest control of the multisig is lost. -### iTG-002: Nested Cancellation +## Requirements and Contraints + +### Nested Cancellation A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be considered before registering that the child safe is rejecting the transaction. @@ -85,7 +114,7 @@ For a given transaction in a root safe within a nested safe setup, a scheduled t To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe scheduled to execute the transaction, or in one of its child safes. ### Module Execution -Safes implement separate Guard and ModuleGuard interfacesm and a Safe enables a separate Guard and ModuleGuard. To make the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: +Safes implement separate Guard and ModuleGuard interfaces and a Safe enables a separate Guard and ModuleGuard. To make the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: - MUST implement both the Guard and ModuleGuard interfaces. - `ITransactionGuard(timelockGuard).checkTransaction(...)` and `IModuleGuard(timelockGuard).checkModuleTransaction(...)` MUST execute the same delay checking logic. @@ -169,4 +198,4 @@ Called by anyone, verify that the `cancellation_threshold` has been met for the - MUST revert if the TimelockGuard is not enabled for the safe. - MUST revert if `scheduling_time(safe, tx) + delay_period(safe) >= block.timestamp`. - MUST revert if `sum(rejecting_owners(safe, tx)) < cancellation_threshold(safe)`. -- MUST emit a `TransactionCancelled` event, with at least `safe` and a transaction identifier. +- MUST emit a `TransactionCancelled` event, with at least `safe` and a transaction identifier. \ No newline at end of file From 292590eae0cbe4ae1e8dbd2368b87939b57bc1db Mon Sep 17 00:00:00 2001 From: alcueca Date: Thu, 28 Aug 2025 18:21:08 +0200 Subject: [PATCH 6/7] lint and toc --- specs/governance/timelock-guard.md | 167 ++++++++++++++++++++++------- 1 file changed, 126 insertions(+), 41 deletions(-) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index 1fb0e186e..e8d69f7ca 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -6,149 +6,221 @@ - [Overview](#overview) - [Definitions](#definitions) - - [`quorum(safe)`](#quorumsafe) + - [Quorum](#quorum) + - [Blocking Threshold](#blocking-threshold) + - [Active Owner](#active-owner) + - [Honest Owner](#honest-owner) + - [Malicious Owner](#malicious-owner) + - [Full Key Control](#full-key-control) + - [Joint Key Control](#joint-key-control) + - [Temporary Key Control](#temporary-key-control) + - [Multisig Liveness Failure](#multisig-liveness-failure) + - [Multisig Safety Failure](#multisig-safety-failure) - [Scheduled Transaction](#scheduled-transaction) - - [`scheduling_time(safe, tx)`](#scheduling_timesafe-tx) - - [`delay_period(safe)`](#delay_periodsafe) + - [Scheduling Time](#scheduling-time) + - [Delay Period](#delay-period) - [Rejected Transaction](#rejected-transaction) - - [`rejecting_owners(safe, tx)`](#rejecting_ownerssafe-tx) - - [`cancellation_threshold(safe)`](#cancellation_thresholdsafe) - - [Nested Safe Setup](#nested-safe-setup) - - [Nested Cancellation](#nested-cancellation) + - [Rejecting Owners](#rejecting-owners) + - [Cancellation Threshold](#cancellation-threshold) - [Assumptions](#assumptions) + - [aTG-001: Dishonest Users Don't Have Permanent Key Control Over a Quorum of Keys](#atg-001-dishonest-users-dont-have-permanent-key-control-over-a-quorum-of-keys) + - [Severity: Critical](#severity-critical) - [Invariants](#invariants) + - [iTG-001: Honest Users Can Recover From Temporary Key Control Over a Quorum of Keys](#itg-001-honest-users-can-recover-from-temporary-key-control-over-a-quorum-of-keys) + - [Severity: Critical](#severity-critical-1) +- [Requirements and Contraints](#requirements-and-contraints) + - [Nested Cancellation](#nested-cancellation) + - [Module Execution](#module-execution) + - [Replayability Conflicts](#replayability-conflicts) - [Function Specification](#function-specification) - - [constructor](#constructor) - - [enable(delay_period)](#enabledelay_period) + - [enable](#enable) - [disable](#disable) - - [cancellationThreshold(safe)](#cancellationthresholdsafe) - - [delayPeriod(safe)](#delayperiodsafe) - - [setDelayPeriod(delay_period)](#setdelayperioddelay_period) + - [cancellationThreshold](#cancellationthreshold) + - [delayPeriod](#delayperiod) - [scheduleTransaction](#scheduletransaction) - [checkTransaction](#checktransaction) + - [checkModuleTransaction](#checkmoduletransaction) + - [checkPendingTransactions](#checkpendingtransactions) - [rejectTransaction](#rejecttransaction) + - [rejectTransactionWithSignature](#rejecttransactionwithsignature) - [cancelTransaction](#canceltransaction) ## Overview -The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a Safe Guard. Safe Guards are external contracts that Safes call into before and after the execution of a transaction to enquire if the execution fulfils conditions implemented in the Guard, and revert if they don't. The `TimelockGuard` is a Safe Guard that implements a mandatory delay on the execution of transactions. +The `TimelockGuard` implements a mandatory delay on Gnosis Safe contracts as a Safe Guard. Safe Guards are external +contracts that Safes call into before and after the execution of a transaction to enquire if the execution fulfils +conditions implemented in the Guard, and revert if they don't. The `TimelockGuard` is a Safe Guard that implements +a mandatory delay on the execution of transactions. ## Definitions -The following list defines variables that will be used in TimelockGuard. None of the variable names are compulsory, and if better names are found, they should be used. + +The following list defines variables that will be used in TimelockGuard. None of the variable names are compulsory, +and if better names are found, they should be used. ### Quorum + The number of owners required to execute a transaction. ### Blocking Threshold -The minimum number of owners not willing to execute a transaction, that by not approving the transaction guarantee that `quorum` is not met. It is defined as `min(quorum, total_owners - quorum + 1)`. + +The minimum number of owners not willing to execute a transaction, that by not approving the transaction guarantee +that `quorum` is not met. It is defined as `min(quorum, total_owners - quorum + 1)`. ### Active Owner + An owner that is able to approve transactions. ### Honest Owner + An owner that is never willing to execute transactions outside of governance processes. ### Malicious Owner + An owner that is willing to execute transactions outside of governance processes. ### Full Key Control + An actor has full key control of an owner if it is solely able to approve transactions as that owner. ### Joint Key Control -An actor has joint key control of an owner if it is able, along with other actors, to approve transactions as that owner. + +An actor has joint key control of an owner if it is able, along with other actors, to approve transactions as that +owner. ### Temporary Key Control -An actor has temporary key control of an owner if it is able to approve a limited set or number of transactions as that owner. + +An actor has temporary key control of an owner if it is able to approve a limited set or number of transactions as +that owner. ### Multisig Liveness Failure -A multisig is considered to be in a liveness failure state if the number of active honest owners is less than the `quorum`, or if the number of malicious active owners is equal or greater than the `blocking_threshold`. + +A multisig is considered to be in a liveness failure state if the number of active honest owners is less than the +`quorum`, or if the number of malicious active owners is equal or greater than the `blocking_threshold`. ### Multisig Safety Failure -A multisig is considered to be in a safety failure state if the number of active malicious owners is equal or greater than `quorum`. + +A multisig is considered to be in a safety failure state if the number of active malicious owners is equal or greater +than `quorum`. ### Scheduled Transaction + A specific transaction that has been stored in the Timelock, for execution from a given safe. ### Scheduling Time -The `scheduling_time(safe, tx)` is the time in seconds of the block in which a transaction was scheduled for execution from a given safe. + +The `scheduling_time(safe, tx)` is the time in seconds of the block in which a transaction was scheduled for execution +from a given safe. ### Delay Period -Given a scheduled transaction, the `delay_period(safe)` is the minimum time in seconds after `scheduling_time(safe, tx)` that execution is allowed. + +Given a scheduled transaction, the `delay_period(safe)` is the minimum time in seconds after +`scheduling_time(safe, tx)` that execution is allowed. ### Rejected Transaction -Given a transaction scheduled to execute from given safe, owners for the safe reject the transaction to signal they think the transaction should not be executed after the `delay_period(safe)`. + +Given a transaction scheduled to execute from given safe, owners for the safe reject the transaction to signal they +think the transaction should not be executed after the `delay_period(safe)`. ### Rejecting Owners -Given a transaction scheduled to execute from given safe, the `rejecting_owners(safe, tx)` are the owners for the safe that rejected the transaction. + +Given a transaction scheduled to execute from given safe, the `rejecting_owners(safe, tx)` are the owners for the safe +that rejected the transaction. ### Cancellation Threshold -The `cancellation_threshold(safe)` is the number of owners that must reject a given transaction for it not to be executed after the `delay_period(safe)`. It is equal to `blocking_threshold(safe)`. -Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, increasing by 1 with each consecutive cancelled by the given safe transaction up to `blocking_threshold(safe)`, resetting to 1 with each successfully executed transaction by the given safe. +The `cancellation_threshold(safe)` is the number of owners that must reject a given transaction for it not to be +executed after the `delay_period(safe)`. It is equal to `blocking_threshold(safe)`. + +Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, increasing by 1 with each consecutive +cancelled by the given safe transaction up to `blocking_threshold(safe)`, resetting to 1 with each successfully +executed transaction by the given safe. ## Assumptions ### aTG-001: Dishonest Users Don't Have Permanent Key Control Over a Quorum of Keys + We assume that dishonest users have at most a temporary joint key control over a quorum of keys. #### Severity: Critical + If this invariant is broken, honest control of the multisig is lost. ## Invariants ### iTG-001: Honest Users Can Recover From Temporary Key Control Over a Quorum of Keys -If an attacker has joint or temporary key control over a quorum of keys, honest users should always be able to recover the account. + +If an attacker has joint or temporary key control over a quorum of keys, honest users should always be able to recover +the account. #### Severity: Critical + If this invariant is broken, honest control of the multisig is lost. ## Requirements and Contraints ### Nested Cancellation -A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. -For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be considered before registering that the child safe is rejecting the transaction. +A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be +multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. + +For a given transaction in a root safe within a nested safe setup, a scheduled transaction could be rejected by owners +in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be +considered before registering that the child safe is rejecting the transaction. -To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe scheduled to execute the transaction, or in one of its child safes. +To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe +scheduled to execute the transaction, or in one of its child safes. ### Module Execution -Safes implement separate Guard and ModuleGuard interfaces and a Safe enables a separate Guard and ModuleGuard. To make the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: + +Safes implement separate Guard and ModuleGuard interfaces and a Safe enables a separate Guard and ModuleGuard. To make +the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: - MUST implement both the Guard and ModuleGuard interfaces. -- `ITransactionGuard(timelockGuard).checkTransaction(...)` and `IModuleGuard(timelockGuard).checkModuleTransaction(...)` MUST execute the same delay checking logic. +- `ITransactionGuard(timelockGuard).checkTransaction(...)` and +`IModuleGuard(timelockGuard).checkModuleTransaction(...)` MUST execute the same delay checking logic. It follows that any module will need to implement an entry point function to `scheduleTransaction`. ### Replayability Conflicts -The TimelockGuard does not control replayability of transactions, which is done by either the safe or a potential module would decide to overwrite the logic. + +The TimelockGuard does not control replayability of transactions, which is done by either the safe or a potential +module would decide to overwrite the logic. ## Function Specification -The following list details the functions that must be included the in TimelockGuard. None of the function names are compulsory, and if better names are found, they should be used. + +The following list details the functions that must be included the in TimelockGuard. None of the function names are +compulsory, and if better names are found, they should be used. ### enable + Called by a Safe, enable the TimelockGuard for it and set the `delay_period`. - MUST emit a `ModuleEnabled` event with at least `delay_period` as a parameter. ### disable + Called by a Safe, disable the TimelockGuard for it. - MUST revert if the TimelockGuard is already disabled for the safe. - MUST emit a `ModuleDisabled` event. ### cancellationThreshold + Returns the `cancellation_threshold` for a given safe. - MUST return 0 if the TimelockGuard is not enabled for the safe. ### delayPeriod + Returns the `delay_period` for a given safe. - MUST return 0 if the TimelockGuard is not enabled for the safe. ### scheduleTransaction -Called by anyone using signatures from Safe owners, registers a transaction in the TimelockGuard for execution after the `delay_period`. + +Called by anyone using signatures from Safe owners, registers a transaction in the TimelockGuard for execution after +the `delay_period`. - MUST revert if the TimelockGuard is not enabled for the safe. - MUST take the same parameters as `execTransaction`. @@ -156,10 +228,13 @@ Called by anyone using signatures from Safe owners, registers a transaction in t - MUST revert if an identical transaction was cancelled. - MUST emit a `TransactionScheduled` event, with at least `safe` and relevant transaction data. -To allow for identical transactions to be scheduled more than once, but requiring different signatures for each one, a `salt` parameter can be included in `data` with the sole purpose of differentiating otherwise identical transactions. +To allow for identical transactions to be scheduled more than once, but requiring different signatures for each one, a +`salt` parameter can be included in `data` with the sole purpose of differentiating otherwise identical transactions. ### checkTransaction -Called by anyone, and also by the Safe in `execTransaction`, verifies if a given transaction was scheduled and the delay period has passed. + +Called by anyone, and also by the Safe in `execTransaction`, verifies if a given transaction was scheduled and the +delay period has passed. - MUST revert if the TimelockGuard is not enabled for the safe. - MUST take the exact parameters from the `ITransactionGuard.checkTransaction` interface. @@ -167,18 +242,24 @@ Called by anyone, and also by the Safe in `execTransaction`, verifies if a given - MUST revert if the scheduled transaction was cancelled. ### checkModuleTransaction -Called by anyone, and also by the Safe in `preModuleExecution`, verifies if a given transaction was scheduled and the delay period has passed. + +Called by anyone, and also by the Safe in `preModuleExecution`, verifies if a given transaction was scheduled and the +delay period has passed. - MUST revert if the TimelockGuard is not enabled for the safe. - MUST take the exact parameters from the `IModuleGuard.checkModuleTransaction` interface. - MUST execute the same internal logic as `checkTransaction`. ### checkPendingTransactions + Called by anyone, returns the list of all scheduled but not cancelled transactions for a given safe. -*Note:* If we want to exclude executed transactions from this list, the TimelockGuard would need to query into the storage of the safe, and if a module that overrides replayability is implemented, the TimelockGuard would need to look into its storage as well. +*Note:* If we want to exclude executed transactions from this list, the TimelockGuard would need to query into the +storage of the safe, and if a module that overrides replayability is implemented, the TimelockGuard would need to look +into its storage as well. ### rejectTransaction + Called by a Safe owner, signal the rejection of a scheduled transaction. - MUST revert if the TimelockGuard is not enabled for the safe. @@ -187,15 +268,19 @@ Called by a Safe owner, signal the rejection of a scheduled transaction. - MUST emit a `TransactionRejected` event, with at least `safe` and a transaction identifier. ### rejectTransactionWithSignature -Called by anyone, using signatures form one or more owners, signal the rejection of a scheduled transaction. Can reuse the `rejectTransaction` function name. + +Called by anyone, using signatures form one or more owners, signal the rejection of a scheduled transaction. Can reuse +the `rejectTransaction` function name. - MUST revert if the TimelockGuard is not enabled for the safe. - MUST reuse the same internal logic as `rejectTransaction`. ### cancelTransaction -Called by anyone, verify that the `cancellation_threshold` has been met for the Safe to cancel a given scheduled transaction. + +Called by anyone, verify that the `cancellation_threshold` has been met for the Safe to cancel a given scheduled +transaction. - MUST revert if the TimelockGuard is not enabled for the safe. - MUST revert if `scheduling_time(safe, tx) + delay_period(safe) >= block.timestamp`. - MUST revert if `sum(rejecting_owners(safe, tx)) < cancellation_threshold(safe)`. -- MUST emit a `TransactionCancelled` event, with at least `safe` and a transaction identifier. \ No newline at end of file +- MUST emit a `TransactionCancelled` event, with at least `safe` and a transaction identifier. From 5f9fd1bd3a88e7840a06381249fa259d869eea78 Mon Sep 17 00:00:00 2001 From: alcueca Date: Wed, 3 Sep 2025 10:44:55 +0100 Subject: [PATCH 7/7] Rewrote requirements as invariants, and removed module execution path --- specs/governance/timelock-guard.md | 51 ++++++++++++------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/specs/governance/timelock-guard.md b/specs/governance/timelock-guard.md index e8d69f7ca..351eb3451 100644 --- a/specs/governance/timelock-guard.md +++ b/specs/governance/timelock-guard.md @@ -131,10 +131,8 @@ that rejected the transaction. ### Cancellation Threshold The `cancellation_threshold(safe)` is the number of owners that must reject a given transaction for it not to be -executed after the `delay_period(safe)`. It is equal to `blocking_threshold(safe)`. - -Alternatively, the `cancellation_threshold(safe)` can start at 1 for each safe, increasing by 1 with each consecutive -cancelled by the given safe transaction up to `blocking_threshold(safe)`, resetting to 1 with each successfully +executed after the `delay_period(safe)`. It starts at 1 for each safe, increasing by 1 with each consecutive +cancelled transacton by the given safe up to `blocking_threshold(safe)`, resetting to 1 with each successfully executed transaction by the given safe. ## Assumptions @@ -158,9 +156,7 @@ the account. If this invariant is broken, honest control of the multisig is lost. -## Requirements and Contraints - -### Nested Cancellation +### iTG-002: Owners Of Child Safes Can Signal Rejection Of Transactions A nested safe setup is a safe (parent safe) in which one or more owners are a Gnosis Safe (child safes). There can be multiple levels of nesting. The one parent safe that is not a child of any other safe is called a root safe. @@ -169,28 +165,32 @@ For a given transaction in a root safe within a nested safe setup, a scheduled t in child safes an arbitrary number of levels away. The `cancellation_threshold(safe)` of a child safe must be considered before registering that the child safe is rejecting the transaction. -To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe -scheduled to execute the transaction, or in one of its child safes. +#### Severity: Medium -### Module Execution +If this invariant is broken, the cancellation flow would be not operational for nested setups, making the timelock less +useful. -Safes implement separate Guard and ModuleGuard interfaces and a Safe enables a separate Guard and ModuleGuard. To make -the `delay_period(safe)` in TimelockGuard mandatory, the TimelockGuard: -- MUST implement both the Guard and ModuleGuard interfaces. -- `ITransactionGuard(timelockGuard).checkTransaction(...)` and -`IModuleGuard(timelockGuard).checkModuleTransaction(...)` MUST execute the same delay checking logic. +### iTG-003: The Signatures For A Cancelled Transaction Can Not Be Reused -It follows that any module will need to implement an entry point function to `scheduleTransaction`. +The signatures for a cancelled transaction can not be reused. -### Replayability Conflicts +#### Severity: Medium -The TimelockGuard does not control replayability of transactions, which is done by either the safe or a potential -module would decide to overwrite the logic. +If this invariant is broken, a malicious user could spam the multisig with transactions that must be cancelled, +causing an impact close to a liveness failure through operational overhead. + +### iTG-004: Only Owners Can Signal Rejection Of Transactions + +To avoid rejection spamming, it must be verified upon rejection that the rejecting owner is an owner in the safe +scheduled to execute the transaction, or in one of its child safes. + +#### Severity: Low + +If this invariant is broken, the event history of the timelock would contain useless events. ## Function Specification -The following list details the functions that must be included the in TimelockGuard. None of the function names are -compulsory, and if better names are found, they should be used. +The following list details the functions that must be included the in TimelockGuard. ### enable @@ -241,15 +241,6 @@ delay period has passed. - MUST revert if `scheduling_time(safe, tx) + delay_period(safe) < block.timestamp`. - MUST revert if the scheduled transaction was cancelled. -### checkModuleTransaction - -Called by anyone, and also by the Safe in `preModuleExecution`, verifies if a given transaction was scheduled and the -delay period has passed. - -- MUST revert if the TimelockGuard is not enabled for the safe. -- MUST take the exact parameters from the `IModuleGuard.checkModuleTransaction` interface. -- MUST execute the same internal logic as `checkTransaction`. - ### checkPendingTransactions Called by anyone, returns the list of all scheduled but not cancelled transactions for a given safe.