-
Notifications
You must be signed in to change notification settings - Fork 854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split Guard into Module and Transaction Guard #755
Comments
4 tasks
akshay-ap
added a commit
that referenced
this issue
May 24, 2024
Fixes #755 Summary of changes: - The PR creates a separate interface for Module guards instead of having a single `Guard` interface for both module transactions and Safe transactions. - The new Module guard interface i.e, [IModuleGuard](https://github.com/safe-global/safe-smart-account/pull/758/files#diff-82762908b9416ddadffb149ee4d25f328078fc27f938d454d8a207aad1ec3839R10) has two functions: 1. `checkModuleTransaction` 2. `checkAfterExecution` - The [updated addresses](https://github.com/safe-global/safe-smart-account/pull/758/files#diff-f567630e82b097ce6631513f19df3108366fc8b80a8de13632297dbd68a6f181R18) in migration contracts are taken from logs from the tests. - Rename interface `Guard` to `ITransactionGuard`. - Fix typo: Rename `ModuleTransasctionDetails` to `ModuleTransactionDetails` Codesize: Main branch: ``` Safe 21210 bytes (limit is 24576) SafeL2 22052 bytes (limit is 24576) ``` This PR (+571 bytes): ``` Safe 21781 bytes (limit is 24576) SafeL2 22623 bytes (limit is 24576) ``` Changes in PR: - [x] Documentation - [x] Fix test cases - [x] Rebase branch - [x] Migration contracts Open for discussion: 1. Rename `Guard` to `ITransactionGuard`? -> Yes 2. Rename `setGuard` function to `setTransactionGuard`? : Impacts Safe interface 3. Rename `ChangedGuard` event to `ChangedTransactionGuard`? Impacts services monitoring this event --------- Co-authored-by: Shebin John <[email protected]> Co-authored-by: Mikhail <[email protected]> Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context / issue
Because of codesize limitations, the
guard
in Safe v1.5.0 was both the module and transaction guard for the account. Now that some contract code space has been freed up, we can have separate configurations for transaction guards and module guards.This has the added benefit that guards are backwards compatible instead of requiring an update for Safe v1.5.0 to implement the additional module guard interface.
Proposed solution
Split module guards into a separate configuration from the transaction guard.
Alternatives
Keep things as is.
Additional context
This was already implemented in https://github.com/safe-global/safe-smart-account/tree/feature/module-tx-guard.
The text was updated successfully, but these errors were encountered: