Skip to content
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

refactor: clean up some of the light account types #828

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

moldy530
Copy link
Collaborator

@moldy530 moldy530 commented Jul 11, 2024

Pull Request Checklist


PR-Codex overview

This PR updates functions and types related to light accounts in the smart contracts.

Detailed summary

  • Renamed getLightAccountVersionDef to getLightAccountVersionForAccount
  • Added a type property in multiOwnerAlchemyClient.ts
  • Updated type definitions for LightAccountVersion
  • Updated function parameters and return types in createAccount.ts
  • Updated function imports and descriptions in multiple files

The following files were skipped due to too many changes: account-kit/smart-contracts/src/light-account/accounts/multiOwner.ts, account-kit/smart-contracts/src/light-account/accounts/base.ts, account-kit/smart-contracts/src/light-account/types.ts, account-kit/smart-contracts/src/light-account/utils.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

vercel bot commented Jul 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
aa-sdk-site ❌ Failed (Inspect) Jul 16, 2024 4:53pm
aa-sdk-ui-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2024 4:53pm

@moldy530
Copy link
Collaborator Author

TODO: need to update the migration guide as well

dphilipson
dphilipson previously approved these changes Jul 15, 2024
Copy link
Contributor

@dphilipson dphilipson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great cleanup, thank you!

TEntryPointVersion
> & {
TLightAccountVersion extends LightAccountVersion<"LightAccount"> = LightAccountVersion<"LightAccount">
> = LightAccountBase<TSigner, "LightAccount", TLightAccountVersion> & {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this is so much better

TLightAccountVersion
> = GetEntryPointForLightAccountVersion<"LightAccount", TLightAccountVersion>
TLightAccountVersion extends LightAccountVersion<"LightAccount"> = LightAccountVersion<"LightAccount">,
TEntryPointVersion extends LightAccountEntryPointVersion<"LightAccount"> = LightAccountEntryPointVersion<"LightAccount">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: why does this type still need a TEntryPointVersion parameter rather than inferring the entry point version from TLightAccountVersion like in the other places? Likewise for CreateMultiOwnerLightAccountParams later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I ran into something by not having it generic here... but can't remember what it was. lemme try and remove this all together and infer it from the version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants