-
Notifications
You must be signed in to change notification settings - Fork 78
chore: Add gasprice overflow check #3636
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
Conversation
|
You can find the image built from this PR at Built from 83dac2b |
|
@Ivansete-status This PR is now ready for review, it is one part of resolving #3643 |
Ivansete-status
left a comment
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 it! 💯
Just added a couple of nitpicks :)
| trace "Gas price overflow detected, capping at maximum int value", | ||
| fetchedGasPrice = fetchedGasPrice, maxInt = high(int) |
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.
Maybe :) ?
| trace "Gas price overflow detected, capping at maximum int value", | |
| fetchedGasPrice = fetchedGasPrice, maxInt = high(int) | |
| warn "Gas price overflow detected, capping at maximum int value", | |
| fetchedGasPrice = fetchedGasPrice, maxInt = high(int) |
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.
updated in ac230ad
d47bbce to
5983c85
Compare
Description
Casting from uint64 to int may cause overflow error.
The register proc in the on_chain groupmanager makes an rpc call to get the gas price (which is uint64), multiplies it by 2 and casts to int to be used in the web3 send method.
Changes
Check for overflow before casting and use max int value if there is an overflow.
relates to #3643