Rework Safe Signer Launchpad #377
Merged
Conversation
This file contains 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
nlordell
requested review from
akshay-ap,
mmv08 and
remedcu
and removed request for
a team
mmv08
reviewed
Apr 19, 2024
This was referenced Apr 26, 2024
|
@remedcu and @akshay-ap - this is ready for review. |
Pull Request Test Coverage Report for Build 9031302657Details
💛 - Coveralls |
akshay-ap
approved these changes
May 7, 2024
|
Approved. The documentation needs to be updated as per new implementation which will be handled in a separate PR. |
Documentation updated in #415 |
This PR does a rework of some of the implementation details of the `SafeECDSASignerLaunchpad` contract in light of some observations from the previous PR #376. Namely, this changes the initialization process to work in a slightly different way: 1. Set the target singleton to a special slot when the entry point executes the `initCode` for the account 2. Signature verification checks that the account is an owner. This has the side-effect that you can initialize an account with multiple ECDSA owners and use any of them to sign the first user operation. 3. Promote the Safe to the singleton that was previously in storage. The main difference with the previous flow is that we no longer have two separate `setup` initializers that we `DELEGATECALL` to. Additionally, we added checks that prevent double initialization as well as reentrency issues in the execution function. In addition, this also opens up a pretty clear path for supporting multiple owners with the launchpad as the account has already undergone "regular" Safe setup. This is relevant for #372.
nlordell
force-pushed
the
308/single-setup
branch
from
772e102
to
1bb7037
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR does a rework of some of the implementation details of the
SafeSignerLaunchpadcontract in light of some observations from the previous PR #376.Namely, this changes the initialization process to work in a slightly different way:
initCodefor the account. Safesetupalso happens at this point, meaning that anyDELEGATECALLto the Safe singleton should work and be valid.The main difference with the previous flow is that we no longer have two separate
setupinitializers that weDELEGATECALLto. Additionally, we added checks that prevent double initialization as well as reentrency issues in the execution function.In addition, this also opens up a pretty clear path for supporting multiple owners with the launchpad as the account has already undergone "regular" Safe setup. This is relevant for #372.
Unit tests in order to reach 100% coverage will be introduced in a follow up.