search

ethereum-optimism / optimism

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

OPSM: Improved architecture assertions #11814

Open blmalone opened 2 months ago

blmalone commented 2 months ago

As per comment raised in the following PR: https://github.com/ethereum-optimism/optimism/pull/11623/files#r1750461307

We should add stricter checking to ensure that proxies definitely have their implementations set before they're returned to a user.

mds1 commented 1 month ago

Another consideration is how we can improve the chain assertions. From @mds1 and @maurelian in https://github.com/ethereum-optimism/optimism/pull/11994#discussion_r1766910643:

I sorted these alphabetically to make it easier to diff the list of checks here with the list of checks in DeployImplementations.s.sol. This made me notice that assertValidL1ERC721Bridge was missing here

yikes.

That does get at the fact that these checks are hard to review and maintain. I can't think of a great way to improve on the situation though. Even if we had working code coverage, these aren't technically 'tests' so it's unclear if that would help us.

Would it make sense to create a ticket for future work to use vm.StateAccesses to ensure that all writes are eventually read during assertions?