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

OPSM: Improved architecture assertions #11814

Closed
blmalone opened this issue Sep 9, 2024 · 2 comments
Closed

OPSM: Improved architecture assertions #11814

blmalone opened this issue Sep 9, 2024 · 2 comments

Comments

@blmalone
Copy link
Contributor

blmalone commented Sep 9, 2024

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.

@blmalone blmalone added the opsm label Sep 9, 2024
@mds1 mds1 removed their assignment Sep 10, 2024
@mds1 mds1 changed the title OPSM: Stricter checking - Proxy getter should check that implementation addresses have been set OPSM: Improved architecture assertions Sep 19, 2024
@mds1
Copy link
Contributor

mds1 commented Sep 19, 2024

Another consideration is how we can improve the chain assertions. From @mds1 and @maurelian in #11994 (comment):

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?

@blmalone
Copy link
Contributor Author

Closing as it's addressed through https://github.com/ethereum-optimism/optimism/pull/13510/files and op-deployer checks.

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