layr-labs / eigenlayer-contracts Goto Github PK

Description

Should contain a brief list of changes for the M1 to M2 transition. This includes:

• Paused to unpaused
• Contract upgrades
• Functions/vars added/removed/changed

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

Description

Refactor unit tests between the EigenPod and EigenPodManager for simplicity and clarity

Action Items

• Update EigenPod-PodManager Unit Tests per standard defined in Test Practices Doc
• Update testing doc with potential invariants to monitor and integration test TODOs

Description

Should contain a brief list of changes for the M1 to M2 transition. This includes:

• Paused to unpaused
• Contract upgrades
• Functions/vars added/removed/changed

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

Changes here:

https://github.com/Layr-Labs/eigenlayer-contracts/pull/255/files

Two examples:

• EigenPodUnitTests inherits from EigenPodTests, both of which have their own test functions
• WithdrawalTests inherits from DelegationTests, both of which have their own test functions

As a result, the tests in EigenPodTests and DelegationTests are getting run twice, cluttering the forge test output.

Remediation

We need to choose one of the test contracts to absorb the other

Remapping are duplicated here

and here

Only one (either) is necessary.

Description

Should lay out the contracts used in the system, as well as detail the deployment config, addresses, pause status, and admin details. Should be useful as an index into the rest of the documentation

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

Might be somewhat blocked by other documentation, but can likely just fill in a few details / update this a bit later.

Referencing discussions in the PR here: #238 (comment)

See also discussion on Slack: https://eigenlabs.slack.com/archives/C05PX94L3H9/p1697144597891359?thread_ts=1697139381.917899&cid=C05PX94L3H9

Describe the bug
When running the command source source-env.sh [CHAIN] as described here and following the installation commands here the command outputs the following

Error: unknown command "e" for "yq"

Did you mean this?
        new

Run 'yq --help' for usage.
Error: unknown command "e" for "yq"

Did you mean this?
        new

Run 'yq --help' for usage.
Error: unknown command "e" for "yq"

Did you mean this?
        new

Run 'yq --help' for usage.
Environment variables set:
CHAIN_ID:
EXECUTOR_MULTISIG:
FOUNDRY_FUZZ_RUNS:

To Reproduce
Steps to reproduce the behavior:

1. Clone the repository
2. Install yq
3. run the command
4. See error

Expected behavior
For the script to properly pull from the config and initalize the env variables

Screenshots

Environment
The error was produced

• Ubuntu on WSL
• Docker
• DevContainer extension from visual studio code

Solution

• Installing yq using the following commands

curl -Lo yq https://github.com/mikefarah/yq/releases/download/v4.11.2/yq_linux_amd64
chmod +x yq

And amending the script to use ./yq instead of yq #!/bin/bash
Or fixing the dockerfile to download the correct version of yq

Background

An EigenPod are cannot currently receive ETH via normal EVM execution. The fact that EigenPods can currently accept ETH from withdrawals is due to the fact that this type of transfer occurs outside of EVM execution.

Problem

There exists many scenarios in which the withdrawal credentials are used as a reference to forward rewards to a validator. When the withdrawal credentials are an EOA, this works fine. But when this is a contract account, the contract must have a receive() and/or fallback() exposed, for example:

    receive() external payable {}

    fallback() external payable {}

Since EigenPods do not have this, any attempt to transfer ETH via EVM transaction execution will revert.

Use-Cases

Example: MEV rewards from running mev-boost with your validator, and setting the fee recipient as your withdrawal address.

Example: Nouns raffle where if a validator has ⌐◨-◨ in their graffiti, they are eligible to be awarded with ETH. The ETH is sent directly to the withdrawal address.

There are an unknown number of future circumstances in which a validator may be eligible for rewards. As these cases grow, having a payable withdrawal address becomes even more important.

Proposal

Add:

receive() external payable {}

along with functionality for EigenPod owners to withdraw (or restake) this ETH.

Description

A large feature to (eventually) be added to EigenLayer post-M2 release is support for payments.
We have had some previous discussions and a stale PR exists with draft interfaces + some implemented logic: #56
This tickets prompts its assignee to help synthesize the previous discussions and work into understandable + digestible docs, and continue to move the discussion forward on implementing this feature.
The primary goals here are deciding on the scope and "high-level" design/spec for payments, for the release following the M2 release.

Action Items

• Summarize past work + discussions in a doc, and share it
• Prepare docs + hold discussions as necessary to agree on scoping and design/spec
• Share the final decisions
• Create tickets for implementation and link them back to the doc capturing final key decisions

Blocking Issues

N/A

Description

Updated documentation, laying out overarching goals, properties, features, etc.

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

The word "latest" has two separate meanings:

• of most recent date (referring to something in the past)
• far on in time; toward the end of a period (referring to something in the future)

The Slasher contract uses latest for both of those meanings in two different variables, which makes it very confusing:

• latestUpdateBlock: latest block in the past
• latestServeUntilBlock: latest (furthest) time in the future

I find changing latestServeUntilBlock to furthestServeUntilBlock or some other synonym would rid of this confusion and make the contract clearer.

Description

We have a need for offchain service to make calls to smart contract and get data. Need to add go bindings for a few contracts and binding generation script

User story
Now, Users have to stake on Lido or Rpl before restaking, which is pretty annoying if they have ETH now.
So, maybe we can provide a function called depositEthIntoStrategy to make user stake ETH on Lido/Rpl and restake on Eigen in a single step.

Actions
If user called depositEthIntoStrategy,

• staking Lido/Rpl(user's option) by calling Lido/Rpl contract
• restaking by our strategy.

Acceptance criteria

• Support stake & restake ETH in a single step.

Description

Updated documentation, laying out overarching goals, properties, features, etc.

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

Description

Would be best to have the middleware contracts in a separate repo. They are currently in the avs-unstable branch. I had to ask Jeff today to merge master into avs-unstable since it had fallen behind by 300+ commits and had a few merge conflicts.

The dependencies with eigensdk and incredible-squaring currently look like:

the deploy script for eigenlayer-core contracts is in master (eigensdk uses the contract addresses stored in the output config file in that branch), and incredible-squaring deploys the middleware contracts (registry contracts are stored in the avs-unstable and avs-unstable-require-opt-into-slashing branches). It would be best if incredible-squaring was pointing to another repo with the contracts, instead of a branch, the middleware contracts' dependency on the core contracts would be more obvious (that new repo would branch to a specific commit of eigenlayer-contracts).

Action Items

• Move middleware contracts into another repo (maybe named eigenlayer-avs-contracts?)
• Move blspubkey compendium to core contracts directory (it should now be shared by all avs teams)
• Make sure to also move tests and deployment scripts

Description

At present, the DelegationManager pushes stake updates to EigenDA, and EigenDA only.
This was initially introduced as a "BandAid" type of solution, to ensure that stake updates to EigenDA would be timely on testnet. However, we will still need to find a solution for other AVSs, for both testnet and mainnet.
It's likely that this solution will be satisfactory for EigenDA as well, but at the moment that is still TBD.
The intention is to eventually remove this coupling regardless, but we need to reach a definitive decision on if this coupling will be removed prior to the M2 mainnet release.

Action Items

• Prepare doc explaining decision to be made
• Meeting to reach / ratify decision
• Share decision with any impacted parties
• Tickets for any next steps (if applicable)

Blocking Issues

As explained in the description, this decision is rather blocked by finding another solution for stake updates. Actually removing these updates will be quick if we decide to get rid of them, so it's fine to procrastinate on this decision for a while until more context is available.

Description

Refactor StrategyManager unit tests for simplicity and clarity

Action Items

• Update StrategyManager Unit Tests per standard defined in Test Practices Doc
• Update testing doc with potential invariants to monitor and integration test TODOs

Description

Refactor DelegationManager unit tests for simplicity and clarity

Action Items

• Update DelegationManager Unit Tests per standard defined in Test Practices Doc
• Update testing doc with potential invariants to monitor and integration test TODOs

https://github.com/Layr-Labs/eigenlayer-contracts/blob/40cbfeaefbb3135bbe0bb0f1dfb8ca4cfbb6784b/src/contracts/middleware/IndexRegistry.sol#L190C1-L190C1

This line is causing an underflow when we pass an operatorIdToSwap that is not actually a currently registered operatorId

1. this should return a more useful error if this is intended behavior
2. Probably should be allowed to pass any operatorId (even 0) if its not needed (for eg if I'm the only operator, or if I'm the last operator already)

We just met with an AVS team and they asked us about the insertAfter parameter of the Slasher.RecordStakeUpdate function.

It currently only mentions

     * @param insertAfter the element of the operators linked list that the currently updating middleware should be inserted after
     * @dev insertAfter should be calculated offchain before making the transaction that calls this. this is subject to race conditions, 
     *      but it is anticipated to be rare and not detrimental.
     */

which is not very clear, and even we forgot what exactly this parameter is for, and couldn't answer this question.

Description

Updated documentation, laying out overarching goals, properties, features, etc.

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A, but should be aware of pending changes to EigenPods that also affect the scope of the StrategyManager.

Description

Refactor EigenPod unit tests for simplicity and clarity

Action Items

• Update EigenPodManager Unit Tests per standard defined in Test Practices Doc
• Update testing doc with potential invariants to monitor and integration test TODOs

My current understanding (correct me if I'm wrong) is that AVS teams that have their own token (aka most of them) will need to deploy a strategy for their token. I've looked through our deploy scripts and haven't found one for this specific purpose. The closest I've found is this one which deploys a strategy for some hardcoded token.

Should we have such a script, or am I thinking about this wrong?

A number of components are unlicensed, including, but not limited to, the below.

Description

Post-mainnet upgrade, look into refactoring the DM and DelayedWithdrawalRouter. We currently have a strategyWithdrawalDelayBlocks mapping in the DM and having the DelayedWithdrawalRouter is a bit redundant.

Action Items

• Remove the DelayedWithdrawalRouter and read from DelegationManager.strategyWithdrawalDelayBlocks(beaconStrategy) instead
• Open PR for review
• Fix unit tests
• Update docs

Blocking Issues

M2 upgrade!

ServiceManager instance variable within the RegistryBase contract is currently set to immutable. This is undesirable
as it would lead to cyclic dependency issues when implementing the ServiceManager which only accepts certain function calls from the Registry. Currently the immutable nature is defined in the VoteWeigherBaseStorage which ends up making it's way to the RegistryBase via Interface implementations and Inheritance.

I think there is a problem with estimateFee function. I tried to send transaction from polygon to arbitrum. At first, the fee was small but as I sent more transactions, the fee grew enormously bigger which made me unable to send transaction.

This would be helpful when the creator is a contract as it would remove the need to call ownerToPod to fetch the pod address following creation.

Description

Update EigenPod docs to relfect the new capella->deneb switch in the EigenPodManager that turns on the deneb-specific proof path

Some context on the change:
As per deneb upgrade, an additional 2 fields are added to the ExecutionPayloadHeader container which means that it merkelizes differently (theres an extra layer to the tree), specifically the height of the merkleized ExecutionPayloadHeader is now 5 instead of 4. These two fields however are not connected to our proofs and do not change them. See the deneb updates here.

This PR adds a switch that allows us to set a deneb fork timestamp twice - once from 0 -> to type(uint64).max and then a second time type(uint64).max -> actual_deneb_fork_timestamp. That way, capella withdrawal proofs work along with deneb withdrawal proofs

Action Items

• Add doc edits
• Get review

Blocking Issues

n/a

User story
As a developer querying the BeaconChainOracle contract I wish to get the latest confirmed oracle timestamp from contract storage because filtering through event logs for this data is unnecessarily complex.

Actions

• Update the BeaconChainOracle contract to hold in storage the latestConfirmedOracleTimestamp
  • the contract would update this variable every time fulfillRequest was successfully called.

Description

Updated documentation, laying out overarching goals, properties, features, etc.

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A. There are some pending changes but this should be accomplishable before the changes are merged.

Description

As part of M2 contract upgrade, we moved migration functionality from the StrategyManager to the DelegationManager. The scripts for migrated the queued withdrawals from the SM to the DM needs to be run after the upgrade is completed.

Action Items

• Run migrations scripts in https://github.com/Layr-Labs/migration-scripts
• Verify correctness of migration with post-migration verification script
• Document transaction hashes
• Communicate to business that migration is completed

Description

A large feature to (eventually) be added to EigenLayer post-M2 release is support for slashing.
We have had a number of previous discussions, and there was a preliminary slashing design that was removed from the contracts (in a couple PRs -- notably #234 and #181) after slashing was ruled out-of-scope for M2.
This tickets prompts its assignee to help synthesize the previous discussions and work into understandable + digestible docs, and continue to move the discussion forward on implementing this feature.
The primary goals here are deciding on the scope and "high-level" design/spec for slashing, for the release following the M2 release.

Action Items

• Summarize past work + discussions in a doc, and share it
• Prepare docs + hold discussions as necessary to agree on scoping and design/spec
• Share the final decisions
• Create tickets for implementation and link them back to the doc capturing final key decisions

Blocking Issues

N/A

Describe the bug

https://github.com/Layr-Labs/eigenlayer-contracts/blob/master/docs/experimental/AVS-Guide.md refers to "middleware folder" in a few places, but the folder doesn't exist

To Reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment
Enter important environment info needed to reproduce the bug.

• [e.g. chrome, safari]
• [e.g. version]

Don't Forget To

• Assign this to a project (our default is eigenlayer)
• Add priority + size estimate
• Set status to New

VoteWeigherBaseStorage contract not compiling because of the following issue:

Source "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol" not found: File not found. Searched the following locations:

I was able to resolve the problem once I imported the right contract, which is

@openzeppelin/contracts/proxy/utils/Initializable.sol"

Please let me know if this is the right contract !

Description

We should have pausing flags for these functions, but they aren't implemented yet.

Action Items

• Implement changes to the contracts on a separate branch
• Update tests and scripts as necessary
• Update documentation to reflect changes
• Get PR reviewed
• Respond to review / make any further changes
• Create ticket for tests
• Merge PR

Blocking Issues

Is anything blocking for this? (Do we need to design and/or build something first?)
N/A

See discussion here: #328 (comment)

TLDR: The Goerli deployment info in the README uses inconsistent number of bytes for the markdown "address preview" used to display addresses. The mainnet deployment info uses a consistent 4 bytes format: 0xAABB...CCDD - i.e. 2 leading bytes and 2 ending bytes.

Writing this issue with the perspective of writing the playgroundAVS operator/staker CLI, so examples will be geared towards this, but this probably applies more generally.

Here is one example of the chain of contracts that I am referring to:
BLSRegistryCoordinatorWithIndices point to BLSPubKeyRegistry which points to BLSPublicKeyCompendium.

The contracts point to the other contracts using the interface type, but the interfaces do not have those contract fields listed. Therefore, this requires a bunch of clumsy typecasting from the interface to the contract itself when chaining these calls, like:

BLSPubkeyRegistry(
    address(
        BLSRegistryCoordinatorWithIndices(
            address(contracts.playgroundAVS.registryCoordinator)
        ).blsPubkeyRegistry()
    )
).pubkeyCompendium().registerBLSPublicKey(...)

Could we just add the contract fiels into the interface and call it a day? Otherwise seems like using interfaces is pointless and we might as well store them typed with the actual contract. More generic question, why do we even need interfaces when those are only used by a single contract?

From discussion here: #238 (comment)

The gist is that mostRecentBalanceUpdateTimestamp can be updated inconsistently depending on the order in which you prove withdrawals.

We've made the temporary choice to push DM updates to eigenDA's stakeRegistry for the upcoming M2 testnet launch.
The code was updated with functions such as

This issue, along with Layr-Labs/eigenlayer-middleware#27, is a request to please please add detailed comments in the code where temporary decisions are made. As I've mentioned before, our contracts are starting to be external facing, and we need to maintain a level of respect for external dev's time, who don't have access to all of our internal docs detailing temporary decisions. These should be explicited in the code, where applies.

User story
As a Developer I want to be able to import and use this repository for integration with EigenLayer and I'm unable to do that if I plan on using a Solidity version that is not =0.8.12.

Actions

• An action item list describing the work to be done
• Link to all of the related tasks needed to complete the feature. See the tasks template.
• Include everything in the definition of done e.g. unit tests and documentation

Acceptance criteria
Update the Solidity version of the smart contracts to floating pragma or create a separate branch and do that so that the developers can import interfaces without needing to copy & paste everything.

Description

Updated documentation, laying out overarching goals, properties, features, etc.

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

Description

Refactor EigenPod unit tests for simplicity and clarity

Action Items

• Update EigenPod unit tests per testing standards doc
• Update testing doc with potential invariants to monitor and integration test TODOs

Description

I took on the challenge to learn more about EigenLayer and drafted a "zap" contract for 1-step ETH deposits in both Lido and RocketPool. This contract uses the depositEthIntoStrategyWithSignature method, allowing users to approve other accounts for depositing on their behalf.

Benefits

• Cost Efficiency: Users enjoy reduced gas costs for deposits, contributing to overall affordability.

• Seamless User Experience: Users can provide ETH directly through the EigenLayer frontend, eliminating the need to navigate away, unless they specifically desire cbETH exposure. This ensures a smooth and uninterrupted user journey.

Implementation

Check out the "zap" contract implementation here on GitHub: https://github.com/0xClandestine/eigenzap/tree/main

Notes

Just a heads up, I put this together today, and while it works, the testing coverage is still quite low. Also, I've created this issue to enhance its visibility, especially considering that issue #21 has been lingering for quite some time.

I'm leaving this issue here as a TODO because I want to understand what's going on in the multiquorums contracts (and get that documentation up and running) before I update this.

Description

Right now the DelegationManager uses strictly ordered nonces for all signature logic -- both signatures coming from stakers and signatures coming from operators' "delegation approvers". We've already acknowledged that this introduces a potential race condition, particularly from the "delegation approver" side -- e.g. if an operator wants to approve a bunch of stakers delegating to them at the same time, it's hard to ensure ordering and there's a lot to manage.
Seems highly preferable to use a "salt" type of input instead for delegation approvals, which should remove this race condition concern. In contrast, since a staker can only delegate to one operator at a time, this change does not seem to make sense for staker signatures, only "delegation approver" signatures.

Action Items

• Implement changes to the contracts on a separate branch
• Update tests and scripts as necessary
• Update documentation to reflect changes
• Get PR reviewed
• Respond to review / make any further changes
• Create ticket for tests
• Merge PR

Blocking Issues

N/A

Description

Should contain a brief list of changes for the M1 to M2 transition. This includes:

• Paused to unpaused
• Contract upgrades
• Functions/vars added/removed/changed

Action Items

• PR with draft documentation
• Get review
• Merge PR

Blocking Issues

N/A

Description

Write unit tests for new strategy config features.
#392

Action Items

• Write tests for different configs for thirdPartyTransfersForbidden
• Write tests for admin setters
• Get @wadealexc @gpsanant to review
• Squash and merge pr into m2-mainnet-fixes

Blocking Issues

None