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

Add storage layout and ABI compatibility checks #339

Open
mds1 opened this issue Oct 2, 2024 · 1 comment
Open

Add storage layout and ABI compatibility checks #339

mds1 opened this issue Oct 2, 2024 · 1 comment

Comments

@mds1
Copy link
Contributor

mds1 commented Oct 2, 2024

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.Z release in the superchain registry (mainnet releases, sepolia releases), then we can fetch the storage layout JSON from https://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
  • 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
@ElliotFriedman
Copy link
Contributor

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

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

2 participants