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

Pro 2160 #34

Merged
merged 5 commits into from
Feb 13, 2024
Merged

Pro 2160 #34

merged 5 commits into from
Feb 13, 2024

Conversation

Jineshdarjee
Copy link
Collaborator

Description

  • Added EtherspotBundler implementation in all the test cases.
  • Added the test cases of the get multiple accounts.
  • Added the test cases of the concurrent userops.

@@ -1,13 +1,16 @@
import * as dotenv from 'dotenv';
dotenv.config(); // init dotenv
import { Factory, PrimeSdk } from '@etherspot/prime-sdk';
import { EtherspotBundler } from 'ethers';
Copy link
Member

Choose a reason for hiding this comment

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

Is this valid? Do we have an EtherspotBundler in ethers package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ch4r10t33r Its new implementation is available in the examples.

Choose a reason for hiding this comment

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

I dont think its right and I am surprised that they have such a thing if exists @Jineshdarjee

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, I'll remove it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Committed updated code

@@ -1,13 +1,16 @@
import * as dotenv from 'dotenv';
dotenv.config(); // init dotenv
import { Factory, PrimeSdk } from '@etherspot/prime-sdk';
import { EtherspotBundler } from 'ethers';
Copy link
Member

Choose a reason for hiding this comment

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

Please see above. I am not sure how this will work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its new implementation is available in the examples.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Committed updated code

Copy link

@vignesha22 vignesha22 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@vignesha22 vignesha22 left a comment

Choose a reason for hiding this comment

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

EtherspotBundler class hasn't been implemented in this PR according to the description of the PR

@Jineshdarjee
Copy link
Collaborator Author

@vignesha22 Added EtherspotBundler implementation in test cases.

@vignesha22 vignesha22 self-requested a review February 13, 2024 09:12
Copy link

@vignesha22 vignesha22 left a comment

Choose a reason for hiding this comment

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

LGTM

@Jineshdarjee Jineshdarjee merged commit e0637d0 into master Feb 13, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants