feat: TimelockGuard #324
feat: TimelockGuard #324
Conversation
introduces the TimelockGuard proposal.
| for execution of the transaction. If a threshold of signers have signed the timer is set to one | ||
| value, if all signers have signed the timer is set to some second (presumably shorter) value. | ||
| 4. After the specified time, anyone can then call `Safe.execTransaction` as normal and execution | ||
| will complete as expected. |
There was a problem hiding this comment.
This will need to be merged somehow with the UnorderedExecutionModule. Maybe the TimelockGuard incorporates the bytes32 hashes from the UnorderedExecutionModule and offers a function different from exectTransaction for execution.
There was a problem hiding this comment.
I think that this is doable simply by ensuring that the params of the scheduled transaction match the transaction being executed whether that is by a module or by the normal flow.
Something to consider though, is that certain versions of the Safe (pre 1.4.0 I believe), do not call to the Guard at all when executing a module transaction. So we will likely need to upgrade some if not all of the Safes we use.
There was a problem hiding this comment.
Since a timelock has to lookup the scheduled transactions, presumably with a hash of data, it's an important design point if the timelock signs for a particular nonce or not.
🟨 We may need to have a way to cancel scheduled transactions. Particularly if the schedules are not for a particular nonce, then a timelocked signature could remains open and ready to use, forever.
🟨 If we have the timelock work noncelessly, we need to ensure that each schedule can only work once, AND that you can schedule the same set of actions again, after it has completed before.
There was a problem hiding this comment.
it's an important design point if the timelock signs for a particular nonce or not.
Absolutely agree. In my thinking the hash used in this guard should be generated with Safe.getTransactionHash() so as to avoid writing new logic outside of the Safe.
🟨 We may need to have a way to cancel scheduled transactions. Particularly if the schedules are not for a particular nonce, then a timelocked signature could remains open and ready to use, forever.
Is that covered below in ### Transaction Cancellation Flow, or do you have something in mind that is missing?
🟨 If we have the timelock work noncelessly, we need to ensure that each schedule can only work once, AND that you can schedule the same set of actions again, after it has completed before.
Agree. Paraphrasing to confirm: "it should be possible to cancel a set of signatures so that they can never be submitted, but the actual transaction action must still remain executable (given a new signature is provided)".
Co-authored-by: Maurelian <[email protected]>
…low implementation
|
Reviewed at Design Review meeting on Wednesday, August 20 |
| a norm. | ||
|
|
||
| ### Time Critical Transactions | ||
| Some multisigs may face scenarios where they need to be able to execute transactions very quickly |
There was a problem hiding this comment.
one scenario for which ive seen a need for time critical transactions is when a mistake is made and some kind of timely surgical recovery is possible. are we making the assumption that the new review period via TimelockGuard delay implies no mistakes? or are we assuming that they can be acceptably managed via Pause?
There was a problem hiding this comment.
Bingo, we realized this as well in our threat modelling yesterday. We might have to rethink the feature.
The Pause does not cover all possible safety incidents, so we need an emergency response path.
| - Optimism Foundation (`LivenessModule` user) | ||
| - Optimism Security Council (`LivenessModule` user) |
There was a problem hiding this comment.
| - Optimism Foundation (`LivenessModule` user) | |
| - Optimism Security Council (`LivenessModule` user) | |
| - Optimism Foundation (`TimelockGuard` user) | |
| - Optimism Security Council (`TimelockGuard` user) |
| <!-- Identify the solution requirements and any additional design constraints from the Context and | ||
| Problem Statement section in a bulleted list. --> | ||
|
|
||
| - Mitigate the impact of a malicious majority of owners capable to execute a single transaction. |
There was a problem hiding this comment.
Elsewhere it was stated that the TimelockGuard cannot mitigate this scenario.
|
|
||
| - Mitigate the impact of a malicious majority of owners capable to execute a single transaction. | ||
| - Mitigate the impact of an honest majority approving a malicious or erroneous transaction. | ||
| - Maintain the ability to execute transactions quickly in a worst-case scenario. <-- NOT ADDRESSED |
There was a problem hiding this comment.
Based on this comment, I think we do want to address this.
| - Maintain the ability to execute transactions quickly in a worst-case scenario. <-- NOT ADDRESSED | |
| - Maintain the ability to execute a predetermined set of emergency transactions quickly in a worst-case scenario. |
I've said "predetermined set of emergency transactions" but it might be the case that we instead want to provide a mechanism for executing arbitrary transactions more quickly under some circumstances, this is important to clarify.
| ### Configuration | ||
|
|
||
| A transaction from the Safe configures the Guard by executing a transaction that optionally sets | ||
| a delay period. If the delay period is not set, the TimelockGuard is enabled with no delay. |
There was a problem hiding this comment.
Is there any point in enabling the TimelockGuard with no delay?
Introduces the TimelockGuard proposal.