-
Notifications
You must be signed in to change notification settings - Fork 206
feat(v5): get SCA addr from EP if factory data provided w/o SCA addr #2296
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
base: moldy/v5-base
Are you sure you want to change the base?
Conversation
🌿 Documentation Preview
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
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.
Pull request overview
This PR enhances smart account address derivation by introducing the ability to compute account addresses from factory data when the account address is not explicitly provided. The key change is adding a getSenderFromFactoryData utility function that calls the EntryPoint contract's getSenderAddress method to derive the counterfactual address.
Key Changes:
- Added
getSenderFromFactoryDatautility function to derive smart account addresses from factory data by calling EntryPoint'sgetSenderAddress - Updated account creation functions (
toLightAccount,toMultiOwnerLightAccount,toModularAccountV2,toMultiOwnerModularAccountV1) to use this new utility when factory data is provided without an account address - Added test coverage for address derivation using factory data across all affected account types
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/smart-accounts/src/utils.ts | Added getSenderFromFactoryData function to derive addresses from factory data via EntryPoint |
| packages/smart-accounts/src/ma-v2/accounts/account.ts | Updated toModularAccountV2 to derive address from factory data when provided without account address |
| packages/smart-accounts/src/ma-v2/accounts/account.test.ts | Added test to verify address derivation from factory data |
| packages/smart-accounts/src/ma-v1/accounts/multi-owner-account.ts | Updated toMultiOwnerModularAccountV1 to use factory data for address derivation |
| packages/smart-accounts/src/ma-v1/accounts/multi-owner-account.test.ts | Added test and updated test helper to support factory data parameters |
| packages/smart-accounts/src/light-account/accounts/multi-owner-account.ts | Updated toMultiOwnerLightAccount to derive address from factory data |
| packages/smart-accounts/src/light-account/accounts/multi-owner-account.test.ts | Added test and updated test helper for factory data address derivation |
| packages/smart-accounts/src/light-account/accounts/account.ts | Updated toLightAccount to use factory data for address computation |
| packages/smart-accounts/src/light-account/accounts/account.test.ts | Added test and updated helper to verify factory data address derivation |
| packages/smart-accounts/src/ma-v2/utils/account.ts | Minor import formatting change |
| docs-site | Updated subproject commit reference |
packages/smart-accounts/src/light-account/accounts/multi-owner-account.test.ts
Show resolved
Hide resolved
adamegyed
left a comment
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 makes sense, but we can technically be a bit stricter for the cases where using getSenderAddress is required:
- If the factory address is still the known, default address, then we should prioritize ABI-decoding the
factoryDataand doing local counterfactual computation, because that lets us skip the async call.
This does make it a bit harder to test, but I think it's technically a better model of when the eth_call is really necessary.
| type Hex, | ||
| } from "viem"; | ||
| import type { SmartAccount } from "viem/account-abstraction"; | ||
| import { type SmartAccount } from "viem/account-abstraction"; |
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.
iirc, having only type imports within a regular import block caused issues for React Native
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.
thx, will change.
the issue w/ RN wasn't necessarily due to importing anything like this though. it was just b/c importing typebox at runtime was breaking things. viem works fine.
Pull Request Checklist
yarn test)sitefolder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change)yarn lint:check) and fix any issues? (yarn lint:write)PR-Codex overview
This PR enhances the handling of factory data for deriving account addresses in the
smart-accountspackage. It introduces new functions to retrieve addresses from factory data, improving the prediction mechanism and ensuring fallback to RPC when necessary.Detailed summary
getLightAccountAddressFromFactoryDataandgetMultiOwnerLightAccountAddressFromFactoryDatafor address prediction.toLightAccountandtoMultiOwnerLightAccountfunctions to utilize new factory data methods.