-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Fix validation uint int sizes #6434
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## 4.x #6434 +/- ##
=======================================
Coverage 89.67% 89.67%
=======================================
Files 214 215 +1
Lines 8222 8234 +12
Branches 2223 2224 +1
=======================================
+ Hits 7373 7384 +11
- Misses 849 850 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
LGTM. Thanks for your contribution
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.
Thanks for contribution
Same in this PR , update is required to fix Conflicting file, I'll merge PR after it, Thanks |
584d31d
to
0e76199
Compare
0e76199
to
47004c7
Compare
…t is not a power of 2
… Zod 'check' function is undefined (due to unsupported type)
47004c7
to
d408325
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.
unit test not passing
Checking |
is the solution to use an older version of web3js? |
Hey there im looking into an issue. @fullkomnun theres an inconsistent integration test failing in the pipelines and i think it stems from the last PR you pushed. The abi and the testcase that is failing. any ideas? |
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.
codecov/patch Failing after 1s — 66.66% of diff hit (target 89.88%)
@luu-alex update, it seems to be an eventslog issue. not related to PR |
|
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.
Looks good, thanks,
I just have suggestions for the error string...
packages/web3-errors/test/unit/__snapshots__/errors.test.ts.snap
Outdated
Show resolved
Hide resolved
Any news about this PR? |
@ngmachado will be merged in today |
Description
Previously an ABI that contained a numeric type (int/uint) of size that is not a power of 2 (such as uint40, int24) would result in the validator / check function being set on Zod as undefined and lead to a cryptic TypeError during validation.
I have setup validators for all valid numeric type sizes ([u]int8, [u]int 16, ... , [u]int256) and throw an error right away if during conversion of JSON scheme to Zod an unsupported type is encountered.
Generated 'single parameter' ABI test cases for all valid sizes.
Type of change
Checklist:
npm run lint
with success and extended the tests and types if necessary.npm run test:unit
with success.npm run test:coverage
and my test cases cover all the lines and branches of the added code.npm run build
and testeddist/web3.min.js
in a browser.CHANGELOG.md
file in the root folder.