-
Notifications
You must be signed in to change notification settings - Fork 0
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: 4337 function #2
Conversation
sanyu1225
commented
Oct 23, 2023
- sendUserOperation
- estimateUserOperationGas
- getUserOperationByHash
- getUserOperationReceipt
- supportedEntryPoints
- generateUserOpHash
package-lock.json
Outdated
"version": "0.1.0", | ||
"lockfileVersion": 2, | ||
"requires": true, | ||
"packages": { |
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.
There are both package-lock and yarn-lock files, you might be using npm and yarn both, I'll recommend to use only one package manager ( in this case yarn ).
test/index.test.ts
Outdated
}); | ||
|
||
// it("should call rpcMethods.estimateUserOperationGas with expected parameters", async () => { |
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.
you will also need to add some E2E tests.
@sanyu1225 are you still working in this PR as its in draft or its ready ? |
@jdevcs I will add some E2E tests, thx. |
@jdevcs I've finished adding the E2E tests and have reviewed everything. The PR is now updated and ready for review. Let me know if there are any changes needed. Thanks! |
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.
Great work!
I have a small suggestion. In addition to Junaid comments.
src/validator/index.ts
Outdated
import { UserOperation } from "../type"; | ||
|
||
export type ValidInputTypes = Uint8Array | bigint | string | number | boolean; | ||
|
||
export const isHexStrict = (hex: ValidInputTypes): boolean => | ||
typeof hex === "string" && /^((-)?0x[0-9a-f]+|(0x))$/i.test(hex); | ||
|
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.
You may use isHexStrict
from web3-utils
, if you like:
import { UserOperation } from "../type"; | |
export type ValidInputTypes = Uint8Array | bigint | string | number | boolean; | |
export const isHexStrict = (hex: ValidInputTypes): boolean => | |
typeof hex === "string" && /^((-)?0x[0-9a-f]+|(0x))$/i.test(hex); | |
import { isHexStrict } from "web3-utils"; | |
import { UserOperation } from "../type"; | |
export type ValidInputTypes = Uint8Array | bigint | string | number | boolean; | |
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, I noticed that the isHexStrict
function from web3-utils
was marked as deprecated in the documentation. So, I've switched to using the equivalent function from web3-validator
instead. d450e29
@sanyu1225 some of CI actions are not passing. |
Hi, @jdevcs The CI black box tests are failing because our latest version isn't published on npm yet. I suggest we merge the current changes and release a new version to fix this. Once updated, we can re-run the CI tests. Let me know if you're okay with this plan! Thanks! |
Hi, I'll suggest to create separate github action for build as compared to tests ( including black box ) . If you want you may do it in another PR? |
Sure, thx |