-
Notifications
You must be signed in to change notification settings - Fork 765
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
Fixes randomStatetest643 exception #235
Conversation
Thanks for the fix. Pls change this line (https://github.com/ethereumjs/ethereumjs-vm/blob/master/tests/tester.js#L16) as well so that the test is run. then rebase against master, and I will approve. |
…, this avoids BN assertion failure when returnFee is bigger than 0x4000000
73b156d
to
919b5d0
Compare
@jwasinger , removed test from skipped tests and rebased against master. |
Thanks |
|
||
// avoid BN assertion failure when returnFee is greater than 0x4000000 | ||
if (returnFee > gasLimit.toNumber()) { | ||
returnFee = gasLimit.toNumber() + 1 |
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.
Should this instead emit an "internal error" here? It would be more straightfoward as this one is causing an OOG below, but it is not clear without reading the next if
.
Add basic API tests for re-exports
Current exception in randomStatetest is because an assertion in the bn.js module, it checks that the number to be added is less than 0x4000000
https://github.com/indutny/bn.js/blob/master/lib/bn.js#L2129
These changes checks returnFee, if its greater than gasLimit set it as gasLimit + 1, this avoids BN assertion failure when returnFee is bigger than 0x4000000 and ending as an OOG exception as expected.
Closes #195