Skip to content

feat: transfer da ownership zeus scripts #1310

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

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

Conversation

pakim249CAL
Copy link

@pakim249CAL pakim249CAL commented Apr 12, 2025

Motivation:

There is a desire to transfer ownership of all EigenDA related contracts to the EigenDA operations multisig. Details on the DA upgrade procedure are documented in the EigenDA repo in this PR

Modifications:

Adds zeus scripts to do the following:

  • Queue transferOwnership of the DA Proxy Admin to the Timelock
  • Transfer ownership of DA contracts at the implementation level
  • Execute the transfer of the DA Proxy Admin

Result:

After the scripts are queued and executed, the core team will no longer have ownership of EigenDA contracts. The EigenDA team will have ownership and upgrade privileges for EigenDA contracts.

bowenli86 and others added 17 commits April 1, 2025 11:33
**Motivation:**

chain_id is currently an github secrets, but not used anywhere

**Modifications:**

remove chain id from secrets and env vars 

**Result:**

one less useless secret, no impact on ci
**Motivation:**

per eigengit, main will be the new canonical branch

**Modifications:**

change push branch from dev to main

**Result:**

ci will be run on PRs against main
**Motivation:**

fork-based pr cannot use secrets or github token

**Modifications:**

disable auto labeler to prepare for enabling fork-based pr

**Result:**

auto labeler is disabled, will be potentially removed in future soon
**Motivation:**

*Explain here the context, and why you're making that change. What is
the problem you're trying to solve.*

**Modifications:**

*Describe the modifications you've done.*

**Result:**

*After your change, what will change.*
**Motivation:**

remove holesky rpc since it's not used anywhere

**Modifications:**

remove holesky rpc

**Result:**

remove holesky rpc
**Motivation:**

make testing of fork based pr easier by splitting workflows require
secrets vs not

**Modifications:**

splitting workflows require secrets vs not into 2 gh workflow files

**Result:**

splitting workflows require secrets vs not into 2 gh workflow files
**Motivation:**

*Explain here the context, and why you're making that change. What is
the problem you're trying to solve.*

**Modifications:**

*Describe the modifications you've done.*

**Result:**

*After your change, what will change.*
**Motivation:**

Address cosmetic issues brought up for the native restaking system

**Modifications:**

1. Updates `withdrawRestakedBeaconChainETH` to note that we don't have
to pass in a whole-gwei amount
2. Removes `bytesLib` from EigenPod. Also moves the file into the test
folder
3. Marks `PAUSED_WITHDRAW_RESTAKED_ETH` as deprecated in
`EigenPodPausingConstants`

**Result:**

Cleaner Contracts.
**Motivation:**

Address doc updates from Cantina Audit. 

**Modifications:**

- `deregisterFromOperatorSets` does not revert due to no try-catch
- Permissionless Strategies added to audit report
- We don't do `+1` on the `effectBlock` for allocations for
instantaneous allocations
- New deposits are immediately subject to slashing
- Removing a strategy from an operatorSet does not allow a pending
deallocation to be instantly completable
- Completing a withdrawal as shares is OK even if the strategy is not on
the whitelist
- Delegating to operators with ultra-low magnitudes can result in loss
of funds
- Edge case regarding withdrawals for an operator blocked by owning
negative shares & a fully slashed strategy
- Negative rebase for strategies

**Result:**

More comprehensive docs.

---------

Co-authored-by: Nadir Akhtar <[email protected]>
**Motivation:**

*Explain here the context, and why you're making that change. What is
the problem you're trying to solve.*

Fix for high sev bug found in Cantina competition.

**Modifications:**

Contract Changes (EPM):
- Safecasting uint256 `depositSharesToRemove` to int256 since this input
is provided by the user through `DelegationManager.queueWithdrawals`

Tests:
- Unit test `testFuzz_removeDepositShares_revert_depositSharesOverflow`
which does not bound fuzzed values of `depositSharesToRemove` and checks
for casting overflow, negative shares, or a valid input
- Removed all `uint224` usage in the EPM unit tests
- Regression test POC showing the inflation of staker shares based on
original finding `testFuzz_deposit_revertOverflow_queueWithdrawal`

Screenshot below is test with the expectedRevert message commented out
and with original unsafe casting implementation in the EPM. We can
clearly see the depositSharesAfter is much larger than originally...
<img width="1884" alt="image"
src="https://github.com/user-attachments/assets/77bbbabc-04d8-4761-a102-820f65089534"
/>


Updated Certora Spec:
- Fixed certora parametric rule `integrityOfRemoveDepositShares` to not
constrain input values and assert invalid input does not affect
depositShares. Updated rule passes here
https://prover.certora.com/output/748112/55c59f0cc8144022afe5c22dd1818b77/?anonymousKey=850e5552efec6219cdf5c386b6dc1d62629138e9

<img width="1372" alt="image"
src="https://github.com/user-attachments/assets/5f26451e-0894-48e0-9b4c-eb48e34ed1b4"
/>

**Result:**

Fix for bug

---------

Co-authored-by: Yash Patil <[email protected]>
**Motivation:**

DSF reset isn't emitting event on a full withdrawal.

**Modifications:**

Added event and shadowed declaration to resolve stack too deep

**Result:**

Easier for front-end to track dsf changes.

---------

Co-authored-by: Michael <[email protected]>
**Motivation:**

Upgrade contracts post Cantina Audit 

**Modifications:**

We upgrade the `DelegationManager`, `EigenPodManager`, and `EigenPod`
based on the following PRs:

- #1270
- #1272
- #1265
- #1271

**Result:**

Preprod/Testnet Upgrade Scripts
**Motivation:**

Update .env.example to drop outdated configs

**Modifications:**


Update .env.example to drop outdated configs

**Result:**

cleaner .env.example
**Motivation:**

Step 3 of the execute script does not match what is in the
`upgrade.json`

**Modifications:**

`3-execute.s.sol` -> `3-executeUpgrade.s.sol`

**Result:**

Seamless E2E zeus script
**Motivation:**

rewrite CONTRIBUTING.md to include new development workflow

**Modifications:**

rewrite CONTRIBUTING.md

**Result:**

rewrite CONTRIBUTING.md
…development workflow (#1303)

**Motivation:**

author MAINTENANCE.md with environment network definitions and
development workflow

**Modifications:**

author MAINTENANCE.md

**Result:**

author MAINTENANCE.md
**Motivation:**

goerli has long been gone, we should remove goerli from all codebase

**Modifications:**

remove goerli from all codebase

**Result:**

remove goerli from all codebase
@pakim249CAL pakim249CAL added the 📜 Chore Maintenance related changes (CI, QOL, etc.). label Apr 12, 2025
fix: modify less files

forge fmt
@pakim249CAL pakim249CAL force-pushed the feat/transfer-ownership-to-da-script branch from 4bac54e to c8ac949 Compare April 12, 2025 01:45
@pakim249CAL pakim249CAL marked this pull request as ready for review April 12, 2025 02:09
Comment on lines 1 to 17
{
"name": "transfer-da-ownership",
"phases": [
{
"type": "multisig",
"filename": "1-queueTransferProxyAdmin.s.sol"
},
{
"type": "multisig",
"filename": "2-transferOwnership.s.sol"
},
{
"type": "multisig",
"filename": "3-executeTransferProxyAdmin.s.sol"
}
]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unsure if this format will work correctly, particularly for the multisig steps.
For my recent work on redeploying the TokenHopper on Holesky, I ended up categorizing the script as an 'upgrade' from the current version to the same version (link: https://github.com/Layr-Labs/EigenHopper/blob/master/script/deploy/init-deploy/upgrade.json). This however might have some difficulties/conflict with the fact that the mainnet version will change between steps (2) and (3) due to the Slashing upgrade.
@jbrower95 would be the expert to consult on this

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the heads up. I will try a fix, and hopefully justin can take a look over and see if it looks good.

Copy link
Collaborator

@ypatil12 ypatil12 left a comment

Choose a reason for hiding this comment

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

Tests pass zeus test script/releases/transfer-da-ownership/*.s.sol --env mainnet --rpcUrl <rpc_url>

The only thing I want to call out here is the fact that the DA multisigs have very small thresholds, as Jeff pointed out. We should get security team approval on this as well as public comms. cc @pakim249CAL @ian-shim @wadealexc @nadir-akhtar

@bowenli86 bowenli86 force-pushed the main branch 3 times, most recently from b6b9c4c to 5ffb1b8 Compare April 21, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 Chore Maintenance related changes (CI, QOL, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants