It would be nice to have asimple timelock on top of my Safe as an additional security layer.
I can think of one puclicly available solution : Zodiac’s delay modifier, but it is more aimed at modules and setting it up for a wallet seems to require a rather convoluted setup.
So I created a simple solution for a Safe Wallet, it is a stateful Guard that once set on a Safe will enforces time locks. I also created a basic UI to manage it, to be installed as a custom Safe App
I welcome comments or requests for information. Don’t play with it on mainnet please!
Thanks for sharing the code. It seems to achieve the same delay functionality with a different twist.
One thing I dont understand with your guard: the approveTxWithDelay function is used to queue transactions that can be approved after a delay without a signature by the guarantor/verifier (what’s the difference btw? both are mentionned). It takes safeTxHash as input which itself requires the nonce.
This sounds like quite a limitation. You must know in advance the nonce at which you want a transaction to be effectively executed? So you cant just keep queueing transaction as needed (=not planned) and execute them when the delay is completed
And a question: the pattern you use suggests you would have the same guard contract for many Safes, possibly all Safes? Saves a lot gas and effort during deployment. Is this a general best practice pattern?
Downside is that it seems incompatible with an upgradable guard - which mine is
The approveTxWithDelay(...) is an escape hatch as mentioned here. This is to ensure only in the case when the guarantor is not approving the transactions. Which, in an ideal scenario, should not happen.
They are the same entity.
In this use case, it made the most sense to have the same guard for all safe in a chain, as the only mapping required is for the approvedHashes(...), which should be unique for each case.
Subjectively, I think guards that are only used for a single safe are built that way, considering the complexity of building one that can be used by multiple safes. And IMO makes perfect sense to avoid unnecessary complexity.
In Safenet’s case, this was a thoughtful decision, as for a guard, upgradeability
introduces the risk of unexpected behaviour change (could even result in DoS for Safe)
Completly agree, actually it was not at all a “thoughtful decision” on my side , I was fed up with having a new state on every fix and I wanted to play with upgradable contracts
If you write a guard that does not process any transaction due to a logical error, then this effectively prevents your safe from processing any transaction.
Hypothetical example: You write a guard that checks if a certain state (storage) of the Safe is set to allow transactions. Then you enable the guard before setting that certain state. Now, if you try to perform any transaction while that certain state is not set, you won’t be able to execute any transaction. And unluckily, you haven’t written an escape hatch as well.
, I was fed up with having a new state on every fix and I wanted to play with upgradable contracts