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

feat: align environments #794

Open
wants to merge 74 commits into
base: dev
Choose a base branch
from
Open

Conversation

MinisculeTarantula
Copy link
Collaborator

@MinisculeTarantula MinisculeTarantula commented Oct 3, 2024

feat: add checker, improve configs, cleanup

  • add Current_Config_Check.s.sol that checks the active governance config

  • fix configs by adding more addresses + renaming for clarity

  • delete (apparently) unused files

Context Added For Reviewers (Nov 1, 2024)

The primary productive script here is https://github.com/Layr-Labs/eigenlayer-contracts/blob/feat-align-environments/script/utils/CurrentConfigCheck.s.sol which was already used to align test environments to mainnet, and contains scripts for checking the existing governance config, simulating change of governance powers to include the Protocol Council, and checks for the desired end-state.

I've also made some productive changes to https://github.com/Layr-Labs/eigenlayer-contracts/blob/feat-align-environments/script/utils/ExistingDeploymentParser.sol

This is a start on a new "from scratch deployment script" but is not functional right now -- I'm unclear on the plan for this, so can either delete it or put work into making it more correct, but I recognize that it should not be left in its present state, and it is not worth reviewing right now.

This had a bunch of conflicts with Zeus-related stuff. I just resolved them via a rebase action but we will likely need to sort things out a second time, in a more proper fashion.

@MinisculeTarantula MinisculeTarantula marked this pull request as draft October 3, 2024 21:20
@MinisculeTarantula MinisculeTarantula marked this pull request as ready for review October 8, 2024 18:33
@MinisculeTarantula
Copy link
Collaborator Author

Update:
This PR could likely use more improvement, but is in a place where I believe it is additive + fairly well-formed, so I am marking it as ready for review, but not explicitly requesting review yet.

MinisculeTarantula and others added 14 commits November 1, 2024 13:37
- add Current_Config_Check.s.sol that checks the active governance config

- fix configs by adding more addresses + renaming for clarity

- delete (apparently) unused files
script is still broken, details still need figuring out
more clearly and accurately describe end-state of governance with Protocol Council

add simulation of timelock actions

split simulation into smaller logical pieces
- updates to desired end state, to reflect more "mirrored" design

- modularize simulation and align with desired end state

- deduplicate code for deployment, simulation, and checks
"EIGEN.owner() != eigenTokenTimelockController");
assertEq(Ownable(address(bEIGEN)).owner(), address(eigenTokenTimelockController),
"bEIGEN.owner() != eigenTokenTimelockController");
assertEq(eigenTokenProxyAdmin.owner(), address(eigenTokenTimelockController),
Copy link
Contributor

Choose a reason for hiding this comment

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

this conflicts with the script/deploy/holesky/Eigen_Token_Deploy.s.sol where the owner is the ops multisig

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could potentially just delete that token deploy script -- it feels like it should become part of the "deploy from scratch" script going forwards, but I think this may not be possible right now due to not being able to compile all contracts at once (conflicting OZ versions, in particular)?


// swapOwner(address previousOwner, address oldOwner, address newOwner)
// TODO: figure out if this is the correct input for all chains or the communityMultisig is correct on some
address previousOwner = address(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a required input, and the call will revert without it being correct.
(Gnosis) Safes keep a weird, internal and not-very-easy-to-query linked-list-esque(?) structure of owners which uses address(1) as one end.
Typically this will be another owner of the Safe, but in general as I understand it it's the other node which will need to be attached to the new node to complete the linked structure.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok cool, as long as the test works

targets[1] = address(EIGEN);
payloads[1] = abi.encodeWithSelector(Ownable.transferOwnership.selector, executorMultisig);
// 3. transfer ownership of bEIGEN token to executorMultisig
targets[2] = address(bEIGEN);
Copy link
Contributor

Choose a reason for hiding this comment

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

move to other function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no? why? this timelock is currently the owner of both EIGEN and bEIGEN, so this is how we will actually perform this action.

script is still broken, details still need figuring out
more clearly and accurately describe end-state of governance with Protocol Council

add simulation of timelock actions

split simulation into smaller logical pieces
- updates to desired end state, to reflect more "mirrored" design

- modularize simulation and align with desired end state

- deduplicate code for deployment, simulation, and checks
I have been told Zeus development will cover this
- add Current_Config_Check.s.sol that checks the active governance config

- fix configs by adding more addresses + renaming for clarity

- delete (apparently) unused files
script is still broken, details still need figuring out
more clearly and accurately describe end-state of governance with Protocol Council

add simulation of timelock actions

split simulation into smaller logical pieces
- updates to desired end state, to reflect more "mirrored" design

- modularize simulation and align with desired end state

- deduplicate code for deployment, simulation, and checks
I have been told Zeus development will cover this
some functions got duplicated in rebasing; this commit deletes the duplicates
MinisculeTarantula added a commit that referenced this pull request Nov 15, 2024
an attempt to recreate and fix #794

modifications had to be made due to changes to configs etc
due to work on Zeus

this also notably excludes deploy scripts for the EIGEN token and TimelockControllers
specifically for Holesky, where they were already used
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.

6 participants