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

Broken for L2 Arbitrum #3

Open
kalepail opened this issue Feb 25, 2022 · 7 comments
Open

Broken for L2 Arbitrum #3

kalepail opened this issue Feb 25, 2022 · 7 comments

Comments

@kalepail
Copy link

I'm assuming maybe assembly isn't supported but would you have any idea why this library doesn't work when deployed on Arbitrum?

@kalepail
Copy link
Author

kalepail commented Feb 25, 2022

After extensive testing I can confirm this library broke at this commit 0e3627c for L2 Arbitrum and I assume other L2s as well.

You can see this working via this code here from commit e78d9fd
https://testnet.arbiscan.io/address/0x5a780eb0ae8006BDE6203BFB1a939fd0266f19B2#readContract

It's broken here with this newer base64.sol code however from commit 4d85607
https://testnet.arbiscan.io/address/0xb1452F0488E46e91EeEf61219fE257f91D5eF516#readContract

@kalepail kalepail changed the title Any idea why this doesn't work on L2 Arbitrum? Broken for L2 Arbitrum Feb 25, 2022
@kalepail
Copy link
Author

Reverting mstore8 back to mstore resolves this issue.

@tempe-techie
Copy link

and I assume other L2s as well.

Just FYI: it's not the issue on Optimism. I assume it's the same for Optimism forks.

@kalepail
Copy link
Author

There's also something here with running the mload through shl(248, mload(...)). Just swapping mstore8 for mstore produces invalid base64 strings. Adding back the shl wrapper gets everything working again.

@Brechtpd
Copy link
Owner

Hey, thanks for looking into this. Seems like Arbitrum may have some problems with mstore8, otherwise not much changed. No idea why this would be.

There's also something here with running the mload through shl(248, mload(...)). Just swapping mstore8 for mstore produces invalid base64 strings. Adding back the shl wrapper gets everything working again.

mstore8 only stores the least significant byte, so for mstore to do the same all bytes need to be shifted to the left by 31 bytes so mstore stores the same byte as mstore8 at the same position (+ 31 useless zero bytes to the right of it). mstore8 a bit more efficient so I switched to just doing that.

@kalepail
Copy link
Author

I have an outstanding ticket with the Arbitrum team. We'll see what they say.
https://bugs.immunefi.com/dashboard/submission/5898

Also and not necessarily related but what's the difference between your library and OpenZeppelin's? https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Base64.sol

They both have the same mstore8 issue for L2 and the same fix (using mstore and shl(248, mload(...)) works on both. Any idea if there's a significant gas difference between yours and theirs?

Thanks for the prompt reply on a Friday 🤯

@Brechtpd
Copy link
Owner

Haven't compared them, but they are pretty much the same so any difference in gas will be extremely small. The OpenZeppelin implementation is based on my code, they just saw that the library was used frequently and wanted to include it directly in their collection of contracts.

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

No branches or pull requests

3 participants