Overhaul how we store / discover fault proofs contract addresses in chain configs & validation/standard packages

[geoknee]

There should now be two delayed WETH addresses—one for the permissioned game and one for the permissionless game.

[mds1]

Just adding some extra details:

• For chains on contract versions earlier than op-contracts/v1.6.0 we still expect a single DelayedWETH
• For each chain on op-contracts/v1.6.0, there should be two DelayedWETH contracts
• The first one is for the FaultDisputeGame, which is the permissionless dispute game. Therefore my suggested name for this is the same name we use in OPStackManager (OPSM): DelayedWETHPermissionlessGame, or DelayedWETHPermissionlessGameProxy to convey that this is a proxy.
• The second one is for the PermissionedDisputeGame, therefore my suggested name here again matches OPSM and is DelayedWETHPermissionedGame or DelayedWETHPermissionedGameProxy
• For OP Mainnet, the addresses are:
  • DelayedWETHPermissionlessGameProxy: 0x82511d494B5C942BE57498a70Fdd7184Ee33B975
  • DelayedWETHPermissionedGameProxy: 0x9F9b897e37de5052cD70Db6D08474550DDb07f39
• And for both, the DelayedWETH implementation address is 0x71e966Ae981d1ce531a7b6d23DC0f27B38409087
• A source for those addresses can be found here: https://github.com/ethereum-optimism/superchain-ops/tree/main/tasks/eth/018-granite-upgrade

[ajsutton]

Just to take a step back - what are we trying to accomplish here? The DelayedWETH used is configured for each game type implementation and can be found by starting from the DisputeGameFactory, loading the implementation and calling weth(). As we go forward there will be more game types added and they may or may not share DelayedWETH instances (and we may go back to a single DelayedWETH). Dispute game implementations are designed to be pluggable but in superchain-registry we seem to be fighting against that and I'm not entirely sure why.

It feels like we're making a rod for our own back by trying to make superchain-registry a secondary source of truth about the deployed contracts rather than having the blockchain be that source of truth. I get verifying the deployed contracts have the code we expect and correct owners etc, I'm less clear about why we are trying to track the specific contract addresses of things like DelayedWETH rather than just finding them via the chain itself (and then verifying the code/ownership etc).

[mds1]

Historically the superchain registry has served as a reference of all contract addresses for a chain, and other tools consume this data and treat it a a source of truth. So what we are trying to accomplish here is keeping the superchain registry up to date, as it's currently not reflective of the OP Mainnet architecture—this means either listing both DelayedWETH contracts, or removing both and simply not mentioning DelayedWETH.

To your point, another way to keep it up to date is only list a minimal set of contract addresses and let consumers make calls to discover the rest of the contracts. If we go this route, the extreme version is only listing the SystemConfig address in the registry since everything else can be discovered from that. There are various middle grounds here with different sets of contracts being listed/unlisted. I know this "onchain discovery" approach has been discussed in the past and from what I recall there are two main issues:

• Worse UX: it's slower for consumers because of extra calls, and they need to know more about the architecture to make the required hops to discover what they're looking for.
• The bigger issue is that this is a breaking change, and my understanding is consumers currently expect to find all contract addresses here.

One caveat is that my understanding here may be out of data depending on the superchain registry product vision, so @geoknee or @tessr may have more insight here

[ajsutton]

Given that superchain-registry is regularly out of date like this (ie after pretty much every upgrade for at least a short period), I don't think it makes a very good source of truth. :)

I'd pretty strongly advocate for reducing the amount of contracts you try to list here to just the main entry points. Just SystemConfig might be a bit too restrictive given things like the OptimismPortal address are very fixed and a very common entry point. Whereas things behind the fault dispute game implementation aren't meant to be an entry point and can and will change without warning so I'd say they shouldn't be listed here.

[mds1]

Given that superchain-registry is regularly out of date like this (ie after pretty much every upgrade for at least a short period), I don't think it makes a very good source of truth. :)

Yea, this is definitely a problem we need to solve. We were discussing this in discord a bit here: https://discord.com/channels/1244729134312198194/1285620551808716963

I'm ok with reducing the amount of proofs-specific contracts listed to whatever proofs team suggests though, as long as there are no downstream implications to consider. Maybe short terms we just add the missing DelayedWETH contracts and medium term we figure out this solution?

[ajsutton]

For proofs, I'd list the DisputeGameFactory and stop there to be honest. IMO given the current listing of DelayedWETH is wrong and it's gone this long without being noticed, I'd lean towards just removing DelayedWETH addresses to solve the inconsistency. But I'm happy to go with what you and George think given you have more context on how the superchain-registry is used than I do.

[ajsutton]

To provide some more background on why DisputeGameFactory is the point to stop - every network upgrade will need to deploy new FaultDisputeGame implementation contracts and these aren't proxied so the address will change frequently. But existing games continue to use the older implementation since you can't change the rules of the game part way through. So listing the game implementation addresses needs frequent updating, but is also potentially misleading since both old and new contracts wind up in use depending on the game. And then things like MIPS.sol, AnchorStateRegistry, DelayedWETH etc all hang off the fault game implementation and are implementation details of those. They may be different for different dispute games. Some of them do use proxies so have more stable addresses but it's definitely not something that's guaranteed and they are expected to be used only in the context of those dispute games.

[mds1]

Thanks for these details, this logic makes sense to me and I'm onboard simplifying the superchain registry maintenance by only storing DisputeGameFactory

As for when to do it, I'll defer to @geoknee and co. here on given that I'm unsure what other tooling the change might impact. However it would be nice to make some change soon, to get this repo back to an accurate state. Perhaps the best path forward here is:

• Immediate: Remove the existing DelayedWETH
• Next: Remove all other proofs contracts except for DisputeGameFactory, but TBD what's needed for this

[geoknee]

I don't think there would be a big impact on downstream things, apart from our own validation checks which reference these contracts via the addresses stored in the chain configs. We check the following relationships:

https://github.com/ethereum-optimism/superchain-registry/blob/818111652e9d06e63adf24a4d49f0218de710acb/validation/standard/standard-config-roles-universal.toml#L29-L37

The checks were introduced here https://github.com/ethereum-optimism/superchain-registry/pull/144. @ajsutton if you think it makes sense we could: keep the checks remove the addresses for the contracts and discover them on chain during the course of the validation check?

[ajsutton]

@geoknee Yes, I think the validation checks would be far more meaningful if the address was discovered onchain - then you know you're actually monitoring the contracts that are actually in use. Given we want to monitor those rules for all game types and what types are deployed can vary from chain to chain, my suggestion would be to iterate through all the supported game types (currently on mainnet that's 0 for permissionless and 1 for permissioned) but the total list is:

CannonGameType       GameType = 0
	PermissionedGameType GameType = 1
	AsteriscGameType     GameType = 2
	AsteriscKonaGameType GameType = 3
	FastGameType         GameType = 254
	AlphabetGameType     GameType = 255

FastGameType and AlphabetGameType are for testing, it may be worth a check to verify they aren't deployed.

The game implementations are stored in a map so can't be iterated unfortunately, but its easy enough to request the implementation of each type to check (and no worse than the current hard coded address approach). So with cast that would be:

cast-mainnet call 0xe5965Ab5962eDc7477C8520243A95517CD252fA9 'gameImpls(uint32)(address)' 0

for at least 0 and 1. Then check it returns 0x0000000000000000000000000000000000000000 for 254 and 255 (and 2 & 3 if you want to ensure they aren't deployed until they're actually governance approved). Game type is actually a uint32 so you won't be able to iterate all values and do a completely exhaustive check. You could check that the respectedGameType in OptimismPortal is one of the types you do monitor though since that's ultimately what matters for withdrawals.

And then to get the DelayedWETH contract:

cast-mainnet call <impl-addr> 'weth()(address)'

and to get AnchorStateRegistry:

cast-mainnet call <impl-addr> 'anchorStateRegistry()(address)'

And for game type 1 you can check the challenger and proposer.

That will monitor the contracts that will be used for all newly created games (which is what is currently being monitored). For existing games you'd have to fetch each game and run the checks on it which would be very expensive.

[geoknee]

Thanks @ajsutton I am planning on tackling this as a part of the Holocene work -- I don't want to update the MIPS address, for example, if the longer term plan is to not store it at all.

I started looking into it in more detail and I am wondering about AnchorStateRegistry and PreImageOracle. Should these addresses be stored? From what I can tell, AnchorStateRegistry points to FaultDisputeGame, so is there an argument of including the former and not the latter?

[ajsutton]

Yeah I think we want to keep recording AnchorStateRegistry - with some of the incident response improvements it will become more central to the system and so will only have one shared across all games (it's proxied already so will be upgraded in place). That said, it's FaultDisputeGame that holds the reference to AnchorStateRegistry not the other way around. Though AnchorStateRegistry does have a reference to both SuperchainConfig and DisputeGameFactory which makes things a little circular currently.

PreimageOracle isn't currently proxied which I think is fine so we may wind up with multiple versions for different games when it changes in the future. I think it's reasonable to omit and just discover it from the dispute games.

[alfonso-op]

@geoknee just wanted to confirm whether you still plan to take this up as part of your Holocene work.

[geoknee]

@alfonso-op I had to sideline it in the end because of other priorities. For Holocene we just worked with the existing pattern.

It is something that might be good to still do before Isthmus.

[janjakubnanista]

Closing since fixed by https://github.com/ethereum-optimism/optimism/issues/14757