ethereum-optimism / optimism

Optimism is Ethereum, scaled.
https://optimism.io
MIT License
5.54k stars 3.19k forks source link

OPSM: Refactor to better support features with different inputs #11783

Closed protolambda closed 4 days ago

protolambda commented 1 week ago

Support the initialize call of the SystemConfigInterop that is deployed in DeployImplementationsInterop, by introducing an OPStackManagerInterop and figuring out how to approach an input-struct that is compatible between interop and the inherited stack-manager (e.g. through composition, or dropping the struct pattern).

mds1 commented 1 week ago

We have agreed that structs for inputs are going to be messy, and not scale well as feature develop continues changing inputs. Therefore, we should remove input structs and move to a setter approach like what we do with the output contracts. (The output contracts do also contain structs that we should consider removing as well, in case this simplifies cases where outputs change). That refactor will take a bit of time, however.

Therefore to avoid blocking interop we will first support the interop contracts using the current struct-based input approach. Instead of doing it the "proper" (messy) way with new input structs, we just assume the dependency manager role is the same as the proxyAdminOwner and therefore the inputs don't need to change. For example, see:

https://github.com/ethereum-optimism/optimism/blob/76ade4d91044368cbb91e7f00a1add18bb9ed43c/packages/contracts-bedrock/src/L1/OPStackManagerInterop.sol#L37-L53

Afterwards, we'll refactor the deploy scripts and their corresponding IO contracts to remove structs and use a setter based approach

mds1 commented 1 week ago

I've updated the title here, since interop is now unblocked by our workaround. A good way to verify this refactor is successful is to enable the interop dependencyManager to be set (we currently hardcode it), and ensure the diff is simple

mds1 commented 5 days ago

11833 did not full resolve this, so reopening