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.

Sign up for GitHub

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

Jump to bottom

Add support for EP7.0 and Etherspot's Modular accounts #126

Merged
merged 11 commits into from
May 30, 2024
Merged

Add support for EP7.0 and Etherspot's Modular accounts #126

merged 11 commits into from
May 30, 2024

Conversation

kaushalrajbacancy
Copy link
Contributor

@kaushalrajbacancy kaushalrajbacancy commented May 28, 2024 •
edited
Loading

Description

  • Added support for EP7.0 and Etherspot's Modular accounts
  • Added installModule function to install module
  • Added uninstallModule function to remove module
  • User can pass any validator as a key value on the estimate function; if no validator is specified, the default will be the multipleOwnerECDSAValidator.

Types of changes

What types of changes does your code introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Further comments (optional)

  • I tested it by installing and uninstalling the module with the address 0x6a00da4DEEf677Ad854B7c14F17Ed9312c2B5fDf into the wallet

@linear Linear
Copy link

linear bot commented May 28, 2024

PRO-2345 Etherspot Prime SDK support for Modular accounts

0xSulpiride
0xSulpiride requested changes May 28, 2024
View reviewed changes
Copy link
Member

@0xSulpiride 0xSulpiride left a comment

Choose a reason for hiding this comment

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

great job! I just have couple comments

src/sdk/base/BaseAccountAPI.ts Outdated Show resolved Hide resolved
src/sdk/base/ERC4337EthersProvider.ts Outdated Show resolved Hide resolved
src/sdk/base/EtherspotWalletAPI.ts Outdated Show resolved Hide resolved
src/sdk/base/EtherspotWalletAPI.ts Outdated Show resolved Hide resolved
src/sdk/base/HttpRpcClient.ts Outdated Show resolved Hide resolved
src/sdk/base/VerifyingPaymasterAPI.ts Show resolved Hide resolved
0xSulpiride
0xSulpiride approved these changes May 29, 2024
View reviewed changes
Copy link
Member

@0xSulpiride 0xSulpiride left a comment

Choose a reason for hiding this comment

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

LGTM. We will need to revisit the SDK after finishing our paymaster backend.

lbw33
lbw33 reviewed May 29, 2024
View reviewed changes
src/sdk/common/constants.ts Outdated Show resolved Hide resolved
lbw33
lbw33 reviewed May 29, 2024
View reviewed changes
src/sdk/common/constants.ts Outdated Show resolved Hide resolved
ch4r10t33r
ch4r10t33r reviewed May 29, 2024
View reviewed changes
Copy link
Member

@ch4r10t33r ch4r10t33r left a comment

Choose a reason for hiding this comment

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

How big is the modular sdk size?

examples/03-transfer-erc20.ts Outdated Show resolved Hide resolved
src/sdk/common/utils/execute-mode.ts Show resolved Hide resolved
@ch4r10t33r
Copy link
Member

Pls address this issue as well #127

lbw33
lbw33 reviewed May 29, 2024
View reviewed changes
Copy link
Contributor

@lbw33 lbw33 left a comment

Choose a reason for hiding this comment

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

Just a few comments

@kaushalrajbacancy
Copy link
Contributor Author

How big is the modular sdk size?

dist folder showing 2.6 MB in my local

@kaushalrajbacancy
Copy link
Contributor Author

Pls address this issue as well #127

I have removed @apollo/client and ApiService as they were not being used anywhere.

ch4r10t33r
ch4r10t33r approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@ch4r10t33r ch4r10t33r left a comment

Choose a reason for hiding this comment

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

LGTM

@kaushalrajbacancy kaushalrajbacancy merged commit 43f8bc3 into v2-modular May 30, 2024
4 of 5 checks passed
@kaushalrajbacancy kaushalrajbacancy deleted the feature/pro-2345-support-for-modular-accounts branch May 30, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbw33 lbw33 lbw33 approved these changes

@0xSulpiride 0xSulpiride 0xSulpiride approved these changes

@ch4r10t33r ch4r10t33r ch4r10t33r approved these changes

@arddluma arddluma Awaiting requested review from arddluma

@vignesha22 vignesha22 Awaiting requested review from vignesha22

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kaushalrajbacancy @ch4r10t33r @0xSulpiride @lbw33 @arddluma