-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: SMA 3 - Appended Single Signer Address #123
Conversation
e5f881a
to
1ad0bd8
Compare
e23a856
to
ba63f8d
Compare
@@ -25,7 +25,7 @@ depth = 10 | |||
[profile.optimized-build] | |||
via_ir = true | |||
test = 'src' | |||
optimizer_runs = 15000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to revisit codesize in the future. It's unfortunate to reduce optimizer runs, but it's still more than enough.
5394a22
to
4ecf2ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Had some comments.
@@ -1,4 +1,4 @@ | |||
// SPDX-License-Identifier: UNLICENSED | |||
pragma solidity ^0.8.25; | |||
|
|||
uint32 constant TEST_DEFAULT_VALIDATION_ENTITY_ID = 1; | |||
uint32 constant TEST_DEFAULT_VALIDATION_ENTITY_ID = type(uint32).max; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause the default validation to coincide with the direct call validation entity ID. Could you double-check that this doesn't break assumptions for those tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I dug into this a good bit, the only time we actually care about the entityId for direct-call validations is to build the entire ModuleEntity-- so the last 4 bytes will be 0xff in both cases, but since we only index with the full ModuleEntity, we're reliant on the module address anyway.
385c116
to
7d713ba
Compare
7d713ba
to
9d43bde
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! The simple refactors to UpgradeableModularAccount
to make this possible are great.
Motivation
The third exploration for a semi-modular account, following up from #121 and #119, this implementation combines both by appending only the original owner's address, while hardcoding the fallback signer logic into the account.
Solution
Append the owner's address to the proxy bytecode at account creation time, then, when a validation equivalent to
ModuleEntity.wrap(bytes24(type(uint192).max))
is passed, opt to use, the fallback validation.This validation first checks if it's enabled, then if there's a fallback signer set in storage, or defaults to the bytecode-appended signer.
This allows for reduced deploy costs without much overhead for regular flows.
A lot is still TBD-- such as what to do about the initializer, and where to put the storage for the fallback signer (as well as whether or not to even allow updating it; since users can just install a standard SingleSigner global validation instead).