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

test: sm-fixes-dev-rebase to sm-fixes #1152

Closed
wants to merge 64 commits into from

Conversation

ypatil12
Copy link
Collaborator

No description provided.

8sunyuan and others added 30 commits January 28, 2025 16:33
* feat: slashing release

* fix(slashing): upgrade script part 4 (#953)

* fix: patch (#956)

* feat: bindings (#960)

* fix: remove numtocomplete interface (#966)

* feat: add share helpers (#964)

* feat: add share helpers

* fix: add deposit scaling factor

* fix: rebase

* fix: slashable window boundaries (#965)

* fix: slashable window boundaries

* test: regression for alm

* test: update withdrawal delay not passed reversion

* test: burning indices

* refactor: switch conditionals

* fix: added unit tests

* test: assert slashable shares in queue

* fix: typos

---------

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

* refactor: small cleanup (#959)

refactor small cleanup

chore: `forge fmt`

fix: `getQueuedWithdrawals` + test

fix: add constructor back

test: `totalQueued` > `withdrawal.strategies.length`

test(wip): `completeQueuedWithdrawals`

currently failing

fix: effectBlock

test(wip): @8sunyuan patch

fix: one flaky test

fix: second flaky test

* refactor: remove max strats list (#968)

* feat: slashing patch upgrade script (#967)

* feat: initial deploy

* feat: slashing patch

* fix non-present upgrade.json

* chore: bindings (#969)

* fix: try catch out of gas edge case (#971)

* chore: slashing consolidated script (#972)

* test: more slashing integration todos (#961)

* test(wip): todos

* fix: dealloc issue

* fix: remaining

* fix: forktest upgrade issue

* test: add `check_Withdrawal_AsShares_State_AfterSlash`

* refactor: cleanup

* fix: ci

* refactor: review changes

* docs: wip slashing docs (#925)

* docs: add slashing docs
* chore: bindings
* docs: fixed commenting and updated queue withdrawal docs
* docs: minor cleanup

---------

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

* refactor: scaled shares accounting (#975)

* fix: correct expected share calc

* chore: bindings

* fix: rounding on failing unit test

* refactor: final slashing cleanup (#982)

* chore: clean comments and naming in dm

* refactor: simplify undelegate method
* feat: removed 0 address check because 0 stakers cant be delegated
* feat: condensed non-staker caller logic

* refactor: remove unnecessary check

* feat: use checks-effects-interactions when completing withdrawals
* feat: remove implicit public method for queuedWithdrawals and impl dedicated getter

* feat: deprecate withdrawer field

* chore: make bindings and clean compile errors

* refactor: redelegate reuses delegateTo and undelegate

* fix: broken integration test

* docs: update to reflect deprecated field

* feat: add getter for stakers withdrawal roots

* chore: remove unused avsd events (#984)

* fix: integration test initialization params (#978)

* fix: initialization params

* fix: roll blocks usage

* fix: `SignatureUtils` construction (#990)

* fix: integration test initialization params (#978)

* fix: initialization params

* fix: roll blocks usage

* fix: `SignatureUtils` construction

---------

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

* fix: readd manual checks (#996)

* fix: readd manual checks

* chore: forge fmt

* feat: slashing 1.0.3 upgrade script (#995)

* feat: add step 1

* feat: step 1 & 2 complete; pending step 3 sanity

* test: add `_validateProxyDomainSeparators`

* feat: add rc validation

---------

Co-authored-by: clandestine.eth <[email protected]>

* fix: update alloc config delay bound (#985)

* fix: update alloc delay bound

* test: remove unnecessary roll

* chore: storage report (#1000)

* docs: shares accounting (#997)

* docs: shares accounting

* docs: fix gh markdown view

* docs: try fix gh again

* docs: cleanup

* docs: edit share accounting

* docs: wrap up share accounting doc

* docs: edit edge cases

---------

Co-authored-by: wadealexc <[email protected]>

* refactor: async burning (#1001)

* refactor: burning

* chore: fmt

* chore: update storage report

* chore: update readme

* refactor: add burnableShares for epm storage

* chore: update storage report

* test: `Snapshots` lib (#1002)

* docs: finish delegation manager docs (#1004)

* docs: finish delegation manager docs

* docs: update docs readme

* docs: permission controller

* fix: small typos

* docs: address feedback

* docs: nit

---------

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

* docs: Strategy Manager slashing updates (#999)

* docs: update StrategyManager docs with slashing delta

* docs: remove references to thirdPartyTransfersForbidden

* docs: update strategy docs to latest
* also various edits to docs and natspec

* chore: fmt and make bindings

---------

Co-authored-by: wadealexc <[email protected]>

* docs: EigenPod Manager slashing updates (#1005)

* docs: complete EigenPodManager for slashing

* docs: add in _beaconChainSlashingFactor state variable note

* docs: finish epm docs

* chore: make bindings

---------

Co-authored-by: wadealexc <[email protected]>

* chore: remove rewardsV2 upgrade script

* chore: use stable foundry version

---------

Co-authored-by: clandestine.eth <[email protected]>
Co-authored-by: Jay <[email protected]>
Co-authored-by: Gautham Anant <[email protected]>
Co-authored-by: Yash Patil <[email protected]>
Co-authored-by: Yash Patil <[email protected]>
Co-authored-by: Alex <[email protected]>
Co-authored-by: Nadir Akhtar <[email protected]>
Co-authored-by: davidironblocks <[email protected]>
Co-authored-by: wadealexc <[email protected]>
Co-authored-by: Nadir Akhtar <[email protected]>
* feat: rewards v2.1

feat: operator centric rewards

feat: add new interfaces

feat(wip): implement `createOperatorSetPerformanceRewardsSubmission`

chore: forge fmt

fix: compile

feat(wip): implement `setOperatorSetOperatorSplit`

fix: review changes

fix: add missing `onlyWhenPaused` + `checkCanCall`

feat(wip): add missing `getOperatorSetPerformanceSplit` + rename internals

test(wip): `setOperatorSetPerformanceSplit`

test(wip): `createOperatorSetPerformanceRewardsSubmission`

- some failing

chore: forge fmt

refactor: renaming

chore: storage report

refactor: review changes

refactor: review changes

fix: gap

refactor: review changes

* fix: storage layout

* chore: make storage-report

* docs: natspec

* refactor: review changes

* refactor: review changes

* refactor: review changes

* feat: rewards v2.1 bindings

* fix: not index operatorSet in events

* refactor: forge fmt

* docs: updated storage reports

* chore: updated bindings

---------

Co-authored-by: 0xrajath <[email protected]>
* feat: storage-diff.sh

* refactor: storage-diff.sh

* refactor: storage-diff.sh

* refactor: storage-diff.sh

* refactor: storage-diff.sh

* refactor: storage-diff.sh
* refactor: ci

refactor: remove gas report

fix: gas report

fix: gas report

refactor: continuous fuzzing

refactor: cleanup

feat: gas reporting

feat: intense continuous fuzzing

refactor: rename workflow files

refactor: renaming

refactor: ci

refactor: renaming

refactor: ci

feat: add `FOUNDRY_PROFILE=lite` to ci

refactor: small `foundry.toml` cleanup

refactor: renaming

fix: continuous fuzzing

* refactor: naming

* refactor: intense runs -> 15000

* refactor: move CONTRIBUTING.md -> .github/

* refactor: reduce dotfiles

* refactor: remove `testHelpers`

* feat: add `PULL_REQUEST_TEMPLATE.md`

* refactor: foundry ci

* fix: foundry ci

* refactor: rename

* feat: mainnet storage diff ci

* refactor: certora ci (remove parallelism)

* refactor: intense runs -> 5000

* refactor: coverage

* test: ci

* refactor: continuous fuzzing

* refactor: note minimum avg coverage

* refactor: coverage

* refactor: coverage

* refactor: coverage

* refactor: coverage

* refactor: coverage

* refactor: coverage

* refactor: move storage-diff.yml

* refactor: foundry.yml

* refactor: foundry.yml

* fix: certora scripts

* fix: certora

* refactor: local deploy workflow -> test

* refactor: rename certora workflow

* refactor: checks

* refactor: name

* refactor: comment out certora.yml until they finish slashing specs

* refactor: review changes

* refactor: ci runs 1 -> 256

* fix: coverage branch filtering

* refactor: add -vvv flag

* refactor: lowercase pr template name

* refactor: move CONTRIBUTING.md

* feat: more storage diff coverage

* docs: improve contributor guide

* refactor: intense workflow

- add message telling the dev it's purpose

* refactor: intense warning

* refactor: add `.github/configs/`

* fix: ci

* refactor: remove useless commitlint config

* refactor: cleanup

* refactor: add color

* refactor: remove useless color flag

* refactor: remove prettier (unused)
* feat: deny compile warnings

* fix: compile warnings

* fix: ci
* test: `BytesLib`

* test: `Snapshots`

* fix: ci

* fix: ci

* fix: ci

* fix: ci
* feat: only throw for errors

- except __gap overwrite

* ci: storage check should fail

* ci: storage check should fail

* perf: optimize storage ci

* fix: remove bad variable

* chore: forge fmt
* feat: add size diff workflow

Needs improvement and to be moved -> Foundry.yml

* fix: ci

* fix: ci

* test: ci

* refactor: improve logging

* chore: remove test variables
* chore: remove husky

* chore: vscode settings json

* chore: cleanup
**Motivation:**  

Since Foundry unit, integration, and fork tests currently run
sequentially, the total runtime is the sum of all tests — roughly
**25–26 minutes**.

See yourself
[here](https://github.com/Layr-Labs/eigenlayer-contracts/actions/runs/13208807194/job/36878141793?pr=1025).

**Modifications:**  

- Parallelized unit, integration, and fork tests to significantly speed
up CI.
- Fixed flaky tests.  

**Result:**  

By running all three test suites in parallel, we reduced the total
runtime to **6 minutes**, achieving a **4× speedup**!

---------

Co-authored-by: clandestine.eth <[email protected]>
**Motivation:**

given continuous fuzzing tests takes too
long (currently ~2.5h ish), we wanna make it only run when a commit is
merged to dev branch, instead of every PR change

it should make the CI passes as a whole a lot faster, and save us huge
money spent on github actions

**Modifications:**

make continuous fuzzing only run when a commit is merged to dev branch,
instead of every PR change
**Motivation:**

Currently there is no way for an AVS implementing these callback
functions of the IAVSRegistrar.sol to verify that the operatorSetIds
belong to their operatorSets. The register/deregister callback functions
are updated to include the address field.
This PR is combining
#1092 and
#1085

Additionally, the try/catch design for deregistering from operatorSets
was impacting state between AVS and core contracts due to not having
proper gas estimates when submitting transactions. Solutions exploring
passing some suitable gas paramter were explored but eventually removing
it entirely was deemed the best solution.

**Modifications:**

- Added `avs()` view function that is checked upon `setAVSRegistrar`
- added `address avsIdentifier` field in the callbacks, this should
match the `avs()` view function on the AVSRegistrar
- Removed try catch on `deregisterForOperatorSets`

**Result:**

AVSRegistrars cannot be used for multiple operatorSets and AVSRegistrars
can doublecheck to verify that a register/deregister callback is
correctly called for their contracts.
Registration state between AVS and Eigenlayer core contracts for
OperatorSets are now done in atomic transactions (if entrypoint is
always through the AllocationManager)
**Motivation:**

Registrar interface needs to be upgraded to unblock integration quickly
as possible

**Modifications:**

Upgrade script for the AllocationManager

**Result:**

Upgrade to testnet Holesky environment will take place
**Motivation:**

use more powerful machine to run CI and speed it up

we've tested in following combo:

- arch: x86 vs arm64
- cpu: 8 vs 16 vs 32 cores 

turns out our CI has best perf on x86 16core. 

**Modifications:**

use X86 16core runner i configured to run all CI jobs

**Result:**

improvements results:

- integration: 10min -> 4min, 2.5X
- forktest: 16min -> 4min, 4X
- intense fuzz tests: 2.5h -> 56min, 2.5X
- test storage: 13 min -> 2min, 6.5X
- coverage: 22min -> 8min, 3X
**Motivation:**

since we've removed hardhat, there's no typescript code left, so
tsconfig likely is a legacy file that we can remove

**Modifications:**

remove tsconfig

**Result:**

remove tsconfig
**Motivation:**  

Foundry tests only need to run when specific codebase paths are
modified, rather than for all changes.

For example, tests are not required when only documentation or
deployment scripts are updated.

**Modifications:**  

- Added a list of code paths that trigger Foundry tests when modified.  

**Result:**  

- Speeds up CI for non-core contract changes.  
- Reduces CI costs.

---------

Co-authored-by: clandestine.eth <[email protected]>
**Motivation:**  

After multiple improvements to other parts of our CI, the coverage job
has become the longest-running task, limiting the total CI runtime.

This PR significantly reduces the coverage runtime from **16-18
minutes** (already down from **26 minutes** thanks to a previous
improvement using an x64 16-core runner) to just **2 minutes**—an **8-9x
speedup**.

**Modifications:**  

- Parallelized coverage report generation.  
- Cached Foundry and Cargo artifacts.  
- Optimized Forge arguments.

**Result:**  

The coverage runtime is reduced from **16-18 minutes** (previously **26
minutes**) to **2 minutes**, achieving an **8-9x improvement**.

---------

Co-authored-by: clandestine.eth <[email protected]>
**Motivation:**  

Tired of seeing PRs for minor misspellings—let's fix them all at once.  

**Modifications:**  

- Used [typos-cli](https://github.com/crate-ci/typos) to automatically
correct all misspellings across the repository.

**Result:**  

A typo-free codebase... at least for now. 😅
**Motivation:**  

After several rounds of optimization, the entire CI suite is now limited by the `coverage` job, which in turn is constrained by the `install lcov` step. This step currently takes 3.5 minutes to install `lcov` via `apt`.  

This PR modifies the installation method, switching from `apt` to a prebuilt binary. As a result, the installation time is reduced from 3.5 minutes to just 1 second.  

**Modifications:**  

This PR updates the `lcov` installation process to use a prebuilt binary instead of `apt`.  

**Result:**  

Reduces the installation time from 3.5 minutes to 1 second.
**Motivation:**

after multiple round of optimization, the bottleneck in CI is forge
build now, which takes 1.2-1.5min

this PR added caching for forge build, so that every CI run can restore
latest cache, and only compile solidity files that hv been changed,
which significantly lowers compile time from 1.2-1.5min t0 <1s

**Modifications:**

this PR added caching for forge build, so that every CI run can restore
latest cache, and only compile solidity files that hv been changed

**Result:**

significantly lowers compile time from 1.2-1.5min to <1s

before adding cache:

![image](https://github.com/user-attachments/assets/09b84912-6a22-449d-b065-8fb67ea6b835)


after adding cache:

![image](https://github.com/user-attachments/assets/fb7c20a8-a370-4297-a1bb-82fef99aa4e9)
eigenmikem and others added 29 commits February 20, 2025 17:37
* Slashing integration tests (#1003)

* Validate that users who are slashed fully can redeposit once undelegated

* removing arraylib use

* test for updating eigenpod state -> slash/undelegate

* removing unnecessary logging function

* fixing strategy set

* beacon chain tests in progress

---------

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

* Revert "Slashing integration tests (#1003)" (#1007)

This reverts commit e945d8d.

* integration tests for full eigenlayer slashes

* comment re: beacon chain eth partial deposits

* fix: fixing 0 withdrawal issues (#1019)

* fix: fixing 0 withdrawal issues

* style: white space

* docs: changing description for test

---------

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

* test: withdraw as shares for random subset of strategies fully slashed

* style: removing debugging stubs and updating comment

---------

Co-authored-by: Michael <[email protected]>
…#1031)

* refactor(test): clean up random config and upgrade tests

* test: move test to upgrade tests
* test: improve integration invariants
* also removes unneeded fork logic
* adds checks to some invariants
* fixes some broken tests

* test(integration): enable shared setups
* fix: remove token param from Deposit event and related APIs

* fix: forge fmt

* fix: rebase

* fix: update EigenPodManager and test

* fix: update tests

* fix: update eigenpodmanager tests

* fix: update StrategyManagerMock

* feat: add bindings

* fix: update docs
* feat: better introspection for encumbered magnitude

* chore: bindings

* chore: fix comment and make bindings
* chore: remove unused helper

* chore: bindings
* feat: changing burnableShares to EnumerableMap

* style: linter

* docs: storage docs

* style: natspec and import

* style: lint

* feat: adding view function for cronjob and moving functions

* fix: updating storage gap

* docs: storage slots comment

* feat: new bindings

* docs: updating StrategyManager doc

* docs: bindings

---------

Co-authored-by: Michael <[email protected]>
…1037)

* chore: add view functions for isOperatorSlashable

* feat: add `getAllocatedStake` func

* test: getAllocatedStake

* test: add getMinimumSlashableStake tests

* chore: format

* chore: fmt interface

* chore: remove unnecessary test

* chore: update comment

* chore: bindings

---------

Co-authored-by: Yash Patil <[email protected]>
…1042)

* chore: fix forge nightly release breaking two tests

* test: fix outdated alm tests
* fix: overflow bug for pendingDiff input

* test: add check to regression test

---------

Co-authored-by: wadealexc <[email protected]>
* test: regression tests showing invalid state

* fix: require check and update tests
* fix: ep negative shares bug

* fix: comments

* test: add integration tests for neg shares

* chore: remove logs

* chore: use already calculated delta

* chore: use stable foundry release in CI
…rator (#1051)

* feat: add OperatorSharesSlashed event to track shares slashed per operator

* feat: add unit tests

* fix: add more tests
* feat: add `getSharesFromQueuedWithdrawal`

* test: passing

* refactor(review): improve natspec

* refactor(review): maintain original interface

* test(review): add unit tests

* refactor(review): test empty

* refactor(review): test empty

* refactor(review): remove returned `Withdrawal`

* fix: use operator from `Withdrawal`

* test: use operator from `Withdrawal`

* chore: forge fmt
* docs: slashing factors rounding

* chore: forge fmt
* docs: small slash amounts

* docs: update contract docs

---------

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

Fixes an issue where stakers delegating Beacon Chain ETH from slashed
Eigen Pods were able to delegate more shares than they should.
Specifically, operators now are delegated a staker's
`withdrawableShares` rather than their `depositShares`.

**Modifications:**

- Changed accounting logic on delegation in `DelegationManger.sol`
- `DepositScalingFactor` now resets when a staker withdraws all their
shares, whether through undelegation, redelegation, or a simple
withdrawal
- Changes in `StrategyManager.sol`, `IShareManager.sol`,
`SlashingLib.sol`, and `EigenPodManager.sol` to accommodate new
accounting
- New test files and changes to others to reflect new accounting and
invariants
- Updated `docs/SharesAccounting.md`

**Result:**

System is now robust to stakers with arbitrary EigenPod states

---------

Co-authored-by: Michael <[email protected]>
Co-authored-by: Michael Sun <[email protected]>
Co-authored-by: wadealexc <[email protected]>
Co-authored-by: Yash Patil <[email protected]>
Co-authored-by: clandestine.eth <[email protected]>
**Motivation:**

Audit report flagged that function selector-based permissions may break
on upgrades. This PR documents the limitation and its implications while
improving NatSpec for clarity. (EGSL-15)

**Modifications:**

- Documented function selector upgrade invalidations.
- Improved NatSpec comments in `IPermissionController`.

**Result:**

Clearer documentation on function selector limitations and enhanced
NatSpec for better code clarity.
- *Dynamic Domain Separator:* `SignatureUtils.domainSeparator()` is now
recomputed for each signature verification. This eliminates the need for
storing initial values in storage or as immutables, which is important
for beacon proxy support.

- ~*Version Bump Command:* Introduced `make bump-version VERSION=2`,
which automatically updates the version function's return values.~

- *Version Fn + Constructor Param:* Adds an immutable oz `ShortString`
that's set in the constructor.
**Motivation:**

Concerns about reentrancy in the DelegationManager and interactions of
completed withdrawals which can call untrusted ERC20 transfers

**Modifications:**

Added reentrant guards across external functions

**Result:**

Preventing cross-function reentrancy in the DelegationManager

---------

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

Fixes an issue arbitrary external contracts could be called via
`StrategyManager.burnShares`. (Certora L-04)

**Modifications:**

`StrategyManager.burnShares` does not do an external call if the
burnable share amount is zero

**Result:**

Should no longer be possible to call untrusted code directly through
`burnShares`
**Motivation:**

Document edge cases around BC/AVS Slashing. 

**Modifications:**

Update docs with justification. 

**Result:**

Clear edge case callouts.
require avs register metadata in allocation manager before they can
create operatorset

---------

Co-authored-by: clandestine.eth <[email protected]>
**Motivation:**

Current fn only returns scaled shares, which leads integrators to making
two calls. This is expensive in terms of gas.

**Modifications:**

- `getSharesFromQueuedWithdrawal` has been renamed to
`getQueuedWithdrawalFromRoot` and now also returns `Withdrawal` struct.

**Result:**

Integrators can fetch both in a single call.
**Motivation:**

Improve slashing invariants in integration tests

**Modifications:**

Adds `check_Base_Slashing_State`, and implements several checks used
within

**Result:**

Slashing invariants check all manner of state changes in the ALM and
delegation.

---------

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

Naming between the `getQueuedWIthdrawal` aliases was inconsistent.

**Modifications:**

- ~made `queuedWithdrawals[withdrawalRoot]` mapping public.~
- renamed `queuedWithdrawals` -> `_queuedWithdrawals`.
- added `_queuedWithdrawals` getter
- removed previous `getQueuedWithdrawal` alias.
- renamed `getQueuedWithdrawalFromRoot` to `getQueuedWithdrawal`. 

**Result:**

Consistent function naming.

---------

Co-authored-by: Yash Patil <[email protected]>
@ypatil12 ypatil12 closed this Feb 20, 2025
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.

10 participants