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

fix(Multicall): use correct assembly for revert bubbling #98

Closed
wants to merge 2 commits into from

Conversation

penandlim
Copy link

@penandlim penandlim commented Feb 29, 2024

Related Issue

N/A

Description of changes

image

The implementation taken from stackoverflow uses incorrect assembly.

Since the first 32 bytes of bytes memory result will be the length as uint256, only adding 4 to the pointer makes the Error(string) signature (0x6408c379a0) as the length, which is incorrect. See tests here

This medium article by @0xdeadbeef0x goes over the possible things that can go wrong with this exact implementation:
https://medium.com/@0xdeadbeef0x/the-double-edged-sword-of-abi-decode-f81529e62bcc

Therefore the correct way is to add 32 and revert the data with the length specified.

Also this solution works for both string style (revert("reason"), require(false, "reason")) and custom error style revert CustomError(), making the code cleaner.

@snreynolds
Copy link
Member

something like this has already been implemented, thank you for raising this!

@snreynolds snreynolds closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants