-
Notifications
You must be signed in to change notification settings - Fork 51
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
EVM: respect prank address in callChecks #379
Conversation
Previously we would incorrectly fail here if sending value after calling `vm.prank` since `callChecks` did not take the value of `overrideCaller` into account when checking if the caller had sufficient balance. There is some duplication of the logic in `transfer` here, but I'm not sure how to handle it better, since I do not want `transfer` to be incomplete, and we can't rely on `transfer` for callcode and delegatecall (which don't actually move any eth). This fixes #343.
I'm re-kicking it, seems to have died during fuzzing, either running out of time or out of stack. |
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
looks like we're hitting some failures in |
I'm re-kicking it. That bug you hit is a randomly occurring, known one, I think. I think it'd be worthwhile to get this merged :) Let's see if it works after re-kick. If it does, I'll merge :) |
Description
Previously we would incorrectly fail here if sending value after calling
vm.prank
sincecallChecks
did not take the value ofoverrideCaller
into account when checking if the caller had sufficient balance.There is some duplication of the logic in
transfer
here, but I'm not sure how to handle it better, since I do not wanttransfer
to be incomplete, and we can't rely ontransfer
for callcode and delegatecall (which don't actually move any eth).This fixes #343.
Checklist