Allow passing implementation address to factory #51
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
wilsoncusack
commented
Apr 10, 2024
| @@ -46,12 +42,13 @@ contract CoinbaseSmartWalletFactory { | |||
| } | |||
|
|
|||
| (bool alreadyDeployed, address accountAddress) = | |||
| LibClone.createDeterministicERC1967(msg.value, implementation, _getSalt(owners, nonce)); | |||
| LibClone.createDeterministicERC1967(msg.value, emptyImplementation, _getSalt(owners, nonce, implementation)); | |||
There was a problem hiding this comment.
It's important for security that the original implementation address is in the hash. Otherwise anyone could deploy an account to your address with whatever implementation they want.
Currently this means that a users address across many chains always has to start at the same version in order to have the same address. And so the UX gain is mainly that new users can start at a certain version without needing to upgrade to it.
There was a problem hiding this comment.
Actually the whole EmptyUUPS is moot if we're doing this. May as well just pass implementation here
|
Decided against this:
|
Setland34
pushed a commit
to Setland34/smart-wallet
that referenced
this pull request
Feb 24, 2025
fix(factory): do not revert if account already exists
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.
A single factory with a frozen implementation address was chosen to
implementation.However, the downside of our current approach is that totally new users in the future will create an account and then immediately need to call to upgrade their account to the last version. It would be better if new users could pass the implementation they want to use. They then would need to pass this same implementation on all future chains where they want the same address. This PR enables that.
Supersedes #50