Compose TimelockGuard and LivenessModule2 into a single contract #17402
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: almanax-ai[bot] <174396398+almanax-ai[bot]@users.noreply.github.com>
* Slightly simplified code --------- Co-authored-by: alcueca <[email protected]>
- Add configureTimelockGuard function to allow Safes to set timelock delays - Validate timelock delay between 1 second and 1 year - Check that guard is properly enabled on calling Safe using getStorageAt() - Store configuration per Safe with GuardConfigured event emission - Add comprehensive test suite covering all spec requirements - Implement IGuard interface for Safe compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add clearTimelockGuard function to allow Safes to clear timelock configuration - Validate that guard is disabled before allowing configuration clearing - Check that Safe was previously configured before clearing - Delete configuration data and emit GuardCleared event - Add comprehensive test suite covering all spec requirements - Add new errors: TimelockGuard_GuardNotConfigured, TimelockGuard_GuardStillEnabled 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add internal _getGuard() helper to centralize guard address retrieval - Update configureTimelockGuard and clearTimelockGuard to use helper - Reduces code duplication and improves maintainability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add cancellationThreshold function to return current threshold for a Safe - Return 0 if guard not enabled or not configured - Initialize to 1 when Safe configures guard - Clear threshold when Safe clears guard configuration - Add comprehensive test suite covering all spec requirements - Function never reverts as per spec requirements 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…lity - Add scheduleTransaction placeholder (Function 4) - Add checkPendingTransactions placeholder (Function 6) - Add rejectTransaction placeholder (Function 7) - Add rejectTransactionWithSignature placeholder (Function 8) - Add cancelTransaction placeholder (Function 9) - Update checkTransaction placeholder (Function 5) - All placeholders have proper signatures and documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
maurelian
force-pushed
the
jm/timelock-1
branch
from
c9d5e95 to
e7f2f6a
Compare
maurelian
force-pushed
the
jm/timelock-1
branch
from
e7f2f6a to
0057dd7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## jm/timelock #17402 +/- ##
================================================
+ Coverage 78.34% 89.69% +11.34%
================================================
Files 163 110 -53
Lines 9706 5034 -4672
================================================
- Hits 7604 4515 -3089
+ Misses 1956 519 -1437
+ Partials 146 0 -146
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Also makes the parent contracts abstract in order to prevent deploying them individually. The benefit of this approach is that it ensures the logic is available in both contracts, while ensuring that their state and logic are kept separate thus reducing the complexity and review effort.
maurelian
force-pushed
the
jm/timelock-1
branch
from
0057dd7 to
b62ec78
Compare
alcueca
reviewed
Sep 10, 2025
| /// @dev This contract can be enabled simultaneously as both a module and a guard on a Safe: | ||
| /// - As a module: provides liveness challenge functionality to prevent multisig deadlock | ||
| /// - As a guard: provides timelock functionality for transaction delays and cancellation | ||
| contract SafeExtensions is LivenessModule2, TimelockGuard { |
Contributor
There was a problem hiding this comment.
I liked SaferSafes as the name for the contract :)
Contributor
There was a problem hiding this comment.
It gives a bit more information about what the contract is about, and it is memorable.
Contributor
|
The problem here is that the LivenessModule needs to know the This is inconsistently explained in the specs, let's continue the conversation there. |
maurelian
force-pushed
the
jm/timelock
branch
from
0b47817 to
bc7f865
Compare
Contributor
Author
|
Closing this for now. Will revisit the problem of composition after both modules are complete. |
Closed
Contributor
|
My comment just above is resolved. It's better to not couple the LivenessModule and TimelockGuard features, and make sure that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TimelockGuardandLivenessModule2are inherited into a single contract. The primary purpose of which is to avoid the need to deploy and manage two different contracts.Also makes the parent contracts abstract in order to prevent deploying
them individually.
Since their functionality is otherwise distinct, the child contract contains none of its own logic. The benefit of this approach is that it ensures the logic is available
in both contracts, while ensuring that their state and logic are kept
separate thus reducing the complexity and review effort.
Tests are kept separate for the same reason.