-
Notifications
You must be signed in to change notification settings - Fork 457
[DO NOT MERGE]: Slashing UX Improvements No Formatting #1675
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
Draft
ypatil12
wants to merge
12
commits into
main
Choose a base branch
from
feat/slashing-ux-no-formatting
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**Motivation:** Newly created operators in core have to wait 17.5 days to begin allocating. This is a UX painpoint for new AVSs to onboard their operators. **Modifications:** Update the `AllocationManager` so that the allocation delay for a newly created operator is active immediately block. This allows operators to make an allocation at the very next block. **Result:** Easier onboarding
**Motivation:** We want a centralized way to track protocol contracts, semantic versioning, and improve our pausing procedures. **Modifications:** The main addition is `ProtocolRegistry` which has the following methods: - `ship()` - Batch deployment registration with semantic versioning and configuration flags (only admin). - `configure()` - Configure an existing deployment (only admin). - `pauseAll()` - Emergency pausing across all pausable contracts via single transaction (only pauser). - `version()` - Get the semantic version of the protocol. - `majorVersion()` - Get the major version of the protocol. - `getAddress()` - Get deployment address by contract name. - `getDeployment()` - Get complete deployment info (address, implementation, config) by name. - `getAllDeployments()` - Get all deployments with full information. - `totalDeployments()` - Get total number of registered deployments. **Result:** A registry enabling easy introspection of protocol contracts, improved emergency response capabilities, and centralized semantic versioning (lowering the codesize burden on most of our contracts).
**Motivation:** The `AllocationManager` contract was hitting the 24KB bytecode size limit, which would have blocked deployment. We needed a solution to reduce the contract size while maintaining backwards compatibility with existing integrations that call view functions on `AllocationManager`. **Modifications:** - Created a new `SplitContractMixin` that uses a fallback to delegate unmatched function calls via `delegatecall` to a secondary contract - Split `AllocationManager` into two contracts: the main contract handles state-mutating operations, while `AllocationManagerView` handles all read-only view functions - Both contracts inherit from `AllocationManagerStorage` to share the same storage layout, enabling the view contract to read from the main contract's storage - Updated `AllocationManager` constructor to accept and store the `AllocationManagerView` address - Modified all deployment scripts (devnet, local, integration) to deploy both proxies and implementations - Updated `ExistingDeploymentParser`, test harnesses, and unit/integration tests to work with the split architecture **Result:** - The `AllocationManager` is now about ~ 4.8KB under the 24KB limit with room to grow. - The `AllocationManagerView` contract has 16KB of available space for future view functions. TODO: Add a ci check (I don't have privs but the code is written locally) 1) Get list of all contracts ending in View.sol in the repo. 2) Use forge inspect abi and check all mutability fields == view|pure. 3) Basic search over the file to see if sstore or delegatecall is used on a for additional sanity. --------- Co-authored-by: eigenmikem <michael.muehl@eigenlabs.org>
**Motivation:** We want to reduce codesize throughout our core contracts. **Modifications:** - Removed `SemVerMixin` from every contract that doesn't inherit `SignatureUtilsMixin`. - Added non-revert `checkCanCall` function `PermissionController` for space savings **Result:** ~300 bytes of codesize saved per contract. --------- Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
**Motivation:** We want to solve the following use cases: - As an AVS I want to make a commitment on the address that can slash a given operatorSet - As an AVS I want to allow *one* address to slash an operatorSet - As an AVS I DO NOT want to allow the admin to slash the operatorSet via `PermissionController` actions **Modifications:** The slasher permissions are set and stored in the `AllocationManager` instead of the `PermissionController`. The slasher is initially set upon creation of the operatorSet. New functions: - `updateSlasher`: can set a new slasher on a `ALLOCATION_CONFIGURATION_DELAY` blocks (17.5 days on mainnet) - `migrateSlasher`: migrates the slasher address from the `PermissionController` to the `AllocationManager`. Can be migrated only once for a given operatorSet. This function will be called after the upgrade for all operatorSets. See the script in the script folder, which will eventually be added to the upgrade script. - `createOperatorSet`: modified to take in a slasher address, which can immediately slash an operatorSet. The *old* function is still available and will be deprecated in Q2 2026. The *old* function automatically the slasher to the avs address. Old Functions: - `slashOperator` is modified to not use the `checkCanCall` modifier We have 1.9KB left in the ALM. **Result:** Stronger commitments. --------- Co-authored-by: Nadir Akhtar <nadir-akhtar@users.noreply.github.com>
**Motivation:** The comment of `getSlashableSharesinQueue` in the DM is incorrect. **Modifications:** Fix comment, clarifying that shares return account for the total amount slashable. **Result:** Correct interface.
**Motivation:** Need upgrade scripts for `v1.9.0`. In addition, we want to simplify the amount of copy-pasting we do when writing upgrade. This PR addresses both. **Modifications:** ***v1.9.0:*** Creates two upgrade scripts: 1. `script/releases/v1.9.0-slashing-ux`: Upgrades for non destination chains 2. `script/releases/v1.9.0-slashing-ux-destination`: Upgrades for destination chains (base/base-sepolia) ***CrosschainDeployLib:*** The `CrosschainDeployLib` allows for deterministic deployments, based on a standard `empty` and `proxy` contract byte code. However, this byte code can change based off the metadata hash. In order to have reproducible deploys when metadata hash changes, we hard code the creation code for the empty and proxy contracts. *Note: There are different creation codes for mainnet and testnet environments due to the metadata hash changing between those on-chain deployments.* ***Library Helpers:*** 1. `CoreContractsDeployer`: Helper functions to deploy each implementation core contract 2. `CoreUpgradeQueueBuilder`: Helper functions to add append upgrading each proxy contract to the latest implementation 3. `TestUtils`: Reusable test helpers for deployment 4. `Env`: Updates env to have helper functions that recreate our [deployment matrix](https://github.com/Layr-Labs/eigenlayer-contracts?tab=readme-ov-file#deployments) **Result:** Upgrade scripts + reusable libraries
…ns (#1647) <!-- 🚨 ATTENTION! 🚨 This PR template is REQUIRED. PRs not following this format will be closed without review. Requirements: - PR title must follow commit conventions: https://www.conventionalcommits.org/en/v1.0.0/ - Label your PR with the correct type (e.g., 🐛 Bug, ✨ Enhancement, 🧪 Test, etc.) - Provide clear and specific details in each section --> **Motivation:** An informational finding discovered during a formal verification review points out that `IPermissionController` has three read-only functions not tagged as `view`. This PR rectifies that small discrepancy. **Modifications:** * Addition of `view` modifier to three read-only functions (as shown in `PermissionController`) **Result:** More accurate interfaces --------- Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
**Motivation:**
When developing the slashing release, two different functions for
calculating slashed shares were ideated: `calcSlashedAmount` and
`scaleForBurning`. The former is used to calculated slashed "operator
shares," or shares currently delegated to an operator, and the latter
for "queued shares," or shares in the withdrawal queue that are
currently slashable.
Despite the functionality effectively being equivalent (determine how
many of these shares are to be slashed), two different functions were
maintained due to uncertainty as to how to deduplicate them.
During the redistribution release, manual analysis yielded the
conclusion that there is a way to consolidate these two functions. In
order to reduce disparate logic, this PR removes `scaleForBurning` and
replaces it with `calcSlashedAmount`.
_Context:_
`calcSlashedAmount` and `scaleForBurning` both attempt to do the same
thing: return how many shares to slash as a result of an operator's
magnitude change. However, they each go about it in different ways:
* `calcSlashedAmount` reduces an operator's shares by a certain
proportion, specifically by `newMaxMag / prevMaxMag`, then returns that
number of shares.
* `scaleForBurning` calculates the effective shares at the current and
expected max mags, then returns the difference.
Note that the "type" of share entering each equation is not the same:
`calcSlashedAmount` takes in operator shares, whereas `scaleForBurning`
takes in "scaled shares," i.e. shares that are scaled based on staker
deposit scaling factors (DSFs), but do _not_ change based on operator
slashings.
Once this type discrepancy was evident, the next step was to determine
how to convert from one type to the other. Choosing `calcSlashedAmount`
as the natural base function, it became evident that "scaled shares" can
be translated to operator shares by simply _multiplying by the current
operator magnitude_. With that simple change, `calcSlashedAmount` can be
used to calculate shares slashed from both operator shares as well as
queued shares.
Math:
**Variables**
- $s_n$ - The amount of shares in the storage of the {EP,S}M at time n
- $k_n$ - The “staker deposit scaling factor” at time n. This is
initialized to 1.
- $m_n$ - The operator magnitude at time n
- $op_{n}$ - The shares of the operator $op_{n}=s_{n}k_{n}m_{n}$
- $ss_{n}$ - The scaled shares of the operator in storage. This is
stored upon queueing of a withdrawal $ss_{n}=s_{n}k_{n}$. See
`scaleForQueueWithdrawal` in `SlashingLib`
**Scale For Burning**
$ss_{n}(m_{n}-m_{n+1})$
= $s_nk_n(m_{n}-m_{n+1})$
= $s_nk_nm_{n}- s_nk_nm_{n+1}$
= $op_n - s_nk_nm_{n+1}$
**Calc Slashed Amount**
$op_{n} - op_{n}*\frac{m_{n+1}}{m_n}$
= $op_{n} - s_nk_nm_n*\frac{m_{n+1}}{m_n}$
= $op_{n} - s_nk_n{m_{n+1}}$
**Modifications:**
* Removes `scaleForBurning` from `SlashingLib.sol` and replaces its use
with `calcSlashedAmount`
**Result:**
* Cleaned up code
* Clearer logic
---------
Co-authored-by: Nadir Akhtar <9601075+nadir-akhtar@users.noreply.github.com>
Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
- Release notes for `v1.9.0` - Fix compile issues - Add docs for `ProtocolRegistry` --------- Co-authored-by: Nadir Akhtar <9601075+nadir-akhtar@users.noreply.github.com>
Storage gaps should add up to 50. As demonstrated by inspecting storage via `forge`: ```txt % forge inspect ProtocolRegistryStorage storage ╭--------------------+--------------------------------------------------------------------+------+--------+-------+--------------------------------------------------------------------------------╮ | Name | Type | Slot | Offset | Bytes | Contract | +==================================================================================================================================================================================================+ | _semanticVersion | ShortString | 0 | 0 | 32 | src/contracts/core/storage/ProtocolRegistryStorage.sol:ProtocolRegistryStorage | |--------------------+--------------------------------------------------------------------+------+--------+-------+--------------------------------------------------------------------------------| | _deployments | struct EnumerableMap.UintToAddressMap | 1 | 0 | 96 | src/contracts/core/storage/ProtocolRegistryStorage.sol:ProtocolRegistryStorage | |--------------------+--------------------------------------------------------------------+------+--------+-------+--------------------------------------------------------------------------------| | _deploymentConfigs | mapping(address => struct IProtocolRegistryTypes.DeploymentConfig) | 4 | 0 | 32 | src/contracts/core/storage/ProtocolRegistryStorage.sol:ProtocolRegistryStorage | |--------------------+--------------------------------------------------------------------+------+--------+-------+--------------------------------------------------------------------------------| | __gap | uint256[47] | 5 | 0 | 1504 | src/contracts/core/storage/ProtocolRegistryStorage.sol:ProtocolRegistryStorage | ╰--------------------+--------------------------------------------------------------------+------+--------+-------+--------------------------------------------------------------------------------╯ ``` The `__gap` begins at slot `5` -- however, the gap is of size `47`, which adds up to `52`. Though this is not an issue, it does defy convention, and can cause confusion later if a developer assumes the gap needs to be corrected to add up to 50. --------- Co-authored-by: Nadir Akhtar <9601075+nadir-akhtar@users.noreply.github.com> Co-authored-by: Yash Patil <40046473+ypatil12@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Same as #1670, without the formatting changes