-
Notifications
You must be signed in to change notification settings - Fork 134
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: add alchemy transport update for the mav2 account client #1271
base: main
Are you sure you want to change the base?
feat: add alchemy transport update for the mav2 account client #1271
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label graphite-merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
d05051e
to
b677351
Compare
0bbe272
to
05a7dca
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.
couple nits and just some questions for my own understanding but overall LGTM
We should probs also test the types at a minimum that with AlchemyTransport we get an Alchemy client and any other transport we get a non Alchemy Client
const smaV2Account = await createSMAV2Account({ | ||
...config, | ||
transport, | ||
chain, | ||
}); |
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.
nit: we should be able to just pull this up and remove line 115 below since the account instance can be created the same way regardless of the client
@@ -64,7 +97,21 @@ export function createSMAV2AccountClient< | |||
*/ | |||
export async function createSMAV2AccountClient( | |||
config: CreateSMAV2AccountClientParams | |||
): Promise<SmartAccountClient> { | |||
): Promise<SmartAccountClient | AlchemySmartAccountClient> { |
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.
for my own understanding: does it complain if this is just SmartAccountClient
? technically the AlchemySmartAccountClient is a SmartAccountClient
b677351
to
d05051e
Compare
05a7dca
to
acc1708
Compare
d05051e
to
bfde6c0
Compare
acc1708
to
b826ba0
Compare
bfde6c0
to
136cc8f
Compare
b826ba0
to
73e0725
Compare
136cc8f
to
5d46490
Compare
73e0725
to
60ebd5f
Compare
5d46490
to
18a9228
Compare
60ebd5f
to
0048417
Compare
0048417
to
429ecb6
Compare
Pull Request Checklist
yarn test
)site
folder, 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 focuses on enhancing the
SMAV2AccountClient
functionality by integrating Alchemy support, improving type definitions, and modifying the client creation process to accommodate different transport types.Detailed summary
client.ts
.SMAV2AccountClient
type definitions to support Alchemy transport.CreateSMAV2AlchemyAccountClientParams
type for Alchemy-specific parameters.createSMAV2AccountClient
to handle both standard and Alchemy transports.createSMAV2AccountClient
to return eitherSmartAccountClient
orAlchemySmartAccountClient
.