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

Chain-independent script to generate setImplementation operations #363

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

ajsutton
Copy link
Contributor

Description

Adds a template to the fp-recovery directory with tools to create operations that set the implementation for fault dispute games. Multiple implementations can be set in a single batch (e.g. setting CANNON and PERMISSIONED game type as part of a hard fork upgrade.

This would replace #354 - it generates essentially the same task but in a much more automated way and with the ability to make multiple changes in a single operation.

@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 1, 2024

Tested this on mainnet and it works with a simulation that looks right now that the livenessGuard is automatically determined.

mds1
mds1 previously approved these changes Nov 4, 2024
Copy link
Contributor

@mds1 mds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to rebase against the latest main and re-tested, but once that is done feel free to resolve https://github.com/ethereum-optimism/superchain-ops/pull/363/files#r1828471807 and merge

@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 4, 2024

@mdehoog Could you help me with updating this one to handle the changes in #356 please? Getting the code to compile is easy, but now that it's rebased when I call getOwners on 0x1Eb2fFc903729a0F03966B917003800b145F56E2 (on sepolia L1) which should be the 2-of-2, it only gives me 1 address. The actual contract on L1 still has two owners but it seems the simulation is now removing the council safe as an owner.

It seems when SIMULATE_WITHOUT_LEDGER=true is set, the owners of the 2-of-2 is set to the first owner account of the council safe. But the council safe is still accessed somehow as part of the execution so it fails with "Unallowed Storage access". Was that deliberate? It seems like a weird configuration to be trying to simulate with.

@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 4, 2024

To reproduce the problem I'm hitting run:

just clean prep sepolia op set-implementation 0 0xaA511096675013995A2D58275af6a8880AC39F44
cd out
SIMULATE_WITHOUT_LEDGER=true just --dotenv-path $(pwd)/.env --justfile ../../../../../nested.just simulate council

@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 4, 2024

I get the same problem without SIMULATE_WITHOUT_LEDGER=true as well and when using the foundation safe.

…p from the owner safe.

Updated overrides mean the owner safe getOwners() method now returns the wrong value and can't be trusted.
@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 5, 2024

Worked around this by going back to using the hard coded council and foundation safe addresses (aa9543e) but it's a shame to have to embed that into the script as it means the script itself will have to be adjusted to deal with 3-of-3 safes and other variants.

Overriding the safe owners still seems kind of suspicious though, especially since it doesn't seem to completely eliminate the original owner safes.

@mdehoog
Copy link
Contributor

mdehoog commented Nov 5, 2024

@ajsutton bug fix is here: base-org/contracts#105, apologies for the regression.

@ajsutton
Copy link
Contributor Author

ajsutton commented Nov 5, 2024

Oh nice. Thanks.

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

Successfully merging this pull request may close these issues.

4 participants