Skip to content
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

Ensure reverts are propagated through the call hierachy #574

Merged

Conversation

cburgdorf
Copy link
Collaborator

@cburgdorf cburgdorf commented Oct 15, 2021

What was wrong?

When contract A calls a method on contract B and that method calls revert then the whole call should fail. That isn't the case right now.

How was it fixed?

  • Catch the succeed bool of the call(..) and revert with the return data in case succeed is false.
  • Added tests

@cburgdorf
Copy link
Collaborator Author

I guess I found the reason why that fails:

(pop((call((gas()), addr, 0, instart, insize, 0, 0))))

(pop((call((gas()), addr, 0, instart, insize, 0, 0))))

As far as I can tell, both of these calls should handle the return value which is a success bool to indicate if the call suceeded or not. I guess we simply need to check this and revert if it fails. The tricky part might be to preserve the revert data (if any) but it looks like we have all the ingredients to do that.

@cburgdorf cburgdorf force-pushed the christoph/fix/revert_from_external_call branch from a646eae to 735fd00 Compare October 15, 2021 15:03
@cburgdorf cburgdorf changed the title Demonstrate bug Ensure reverts are propagates through the call hierachy Oct 15, 2021
@cburgdorf cburgdorf changed the title Ensure reverts are propagates through the call hierachy Ensure reverts are propagated through the call hierachy Oct 15, 2021
@cburgdorf cburgdorf force-pushed the christoph/fix/revert_from_external_call branch from 735fd00 to 35102a2 Compare October 15, 2021 15:39
@cburgdorf cburgdorf force-pushed the christoph/fix/revert_from_external_call branch from 35102a2 to 3018777 Compare October 15, 2021 19:33
@cburgdorf cburgdorf merged commit 314d81f into ethereum:master Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants