base64 icon indicating copy to clipboard operation
base64 copied to clipboard

Broken for L2 Arbitrum

Open kalepail opened this issue 3 years ago • 7 comments

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 avatar Feb 25 '22 16:02 kalepail

After extensive testing I can confirm this library broke at this commit https://github.com/Brechtpd/base64/commit/0e3627ccdb03ac8c7b7db52600d12d6ce4856e1c for L2 Arbitrum and I assume other L2s as well.

You can see this working via this code here from commit https://github.com/Brechtpd/base64/commit/e78d9fd951e7b0977ddca77d92dc85183770daf4 https://testnet.arbiscan.io/address/0x5a780eb0ae8006BDE6203BFB1a939fd0266f19B2#readContract

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

kalepail avatar Feb 25 '22 20:02 kalepail

Reverting mstore8 back to mstore resolves this issue.

kalepail avatar Feb 25 '22 21:02 kalepail

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.

tempe-techie avatar Feb 25 '22 21:02 tempe-techie

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.

kalepail avatar Feb 25 '22 22:02 kalepail

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.

Brechtpd avatar Feb 25 '22 22:02 Brechtpd

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 🤯

kalepail avatar Feb 25 '22 22:02 kalepail

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.

Brechtpd avatar Feb 25 '22 23:02 Brechtpd