Add storage layout and ABI compatibility checks
Some tasks upgrade contract implementations. When this happens we want confidence that (1) the new implementation has a compatible storage layout, and (2) the new ABI is backwards compatible, unless we explicitly choose to allow a breaking change to the ABI (e.g. when deprecating old methods and getters)
Based on the state diff from foundry and Tenderly (ref #338) we should be able to detect when an implementation is changing. We'll need to account for how each of the three proxy types (Proxy.sol, L1ChugsplashProxy.sol, and ResolvedDelegateProxy.sol) stores their implementations to ensure each one is identified.
OpenZeppelin has https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades which we might be able to use directly to save ourselves some effort and gain confidence the implementation of the checks is correct. Even if we can't use it directly, we might be able to pull some code or algorithms from it:
If we have to implement everything ourselves, then for each changed implementation we need to:
- Fetch the old storage layout
- If the old implementation address matches a known
op-contracts/vX.Y.Zrelease in the superchain registry (mainnet releases, sepolia releases), then we can fetch the storage layout JSON fromhttps://github.com/ethereum-optimism/optimism/tree/op-contracts/vX.Y.Z/packages/contracts-bedrock/snapshots/[abi|storageLayout]/ContractName.json - If the old implementation does not match a known release, we may have to fetch the source code from etherscan/blockscout and compile it with the right settings—I don't think either explorer has an API to retrieve the storage layout. We might also be able to use cast here, I believe it has some utilities for getting storage layouts
- If the old implementation address matches a known
- Fetch the new storage layout
- The new storage will be governance approved, so the addresses must be in the superchain registry and therefore we can always use the above "known release" approach
- Compare the ABIs. An initial implementation can be: If the ABIs are identical, or the only change is new methods, check passes. Otherwise, check fails. This is likely not perfect, but it's conservative and we can improve this over time
- Compare the storage layout: This is critical and nuanced check so I'm intentionally leaving out the details for now. We can probably pull an equivalence checking algorithm from existing tooling like hardhat and OZ, which have tools for upgrade safety
Looks like the oz foundry-upgrades package is running the oz-upgrades npm package under the hood https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/main/src/internal/Core.sol#L371-L376
Before this, we should consider another check that lives in the monorepo since that is closer to the source and we can improve the storage layout collision detection there
Some research added to this Discord thread: https://discord.com/channels/1244729134312198194/1354110492460711986/1354110500484681819 - Implementation still needs to be completed for this issue.
Some thoughts:
- Check if the slot being updated is an implementation address update (3 proxy types)
- For the current state diff address, get the implementation contract address (check this is equal to the OLD value in the current state diff)
- Use this address to find the old op-contracts release in the superchain-registry
- Using the release, grab the storage layout for the old implementation.
- Now do the same for the NEW value in the current state diff.
Which storage layout do I use for a task that doesn’t update any implementations? Seems like the storageLayout should be dynamically looked up for each contract. i.e. get the contracts impl then reverse lookup the storage layout that way. If it's not a proxy, then simply try to find the address in the superchain-registry standard versions toml file. In failing both of those options, we can default to the latest storageLayout on main. I believe it's safe to do this e.g. contracts like GnosisSafe.