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: parallelize all tests workflow #1074

Closed
wants to merge 54 commits into from
Closed

feat: parallelize all tests workflow #1074

wants to merge 54 commits into from

Conversation

bowenli86
Copy link
Member

@bowenli86 bowenli86 commented Feb 7, 2025

Motivation:

parallelize unit, integration and fork tests to make CI time significantly faster

note currently tho the github action is called "testinparallel", unit, integration and fork tests are not parallelized

their time:

  • unit test: 2-3 min
  • integration test: 9-10 min
  • fork test: 12-13 min

they run in sequence so the total time is sum(all tests) which is roughly 25-26 min. See https://github.com/Layr-Labs/eigenlayer-contracts/actions/runs/13208807194/job/36878141793?pr=1025 for example

by parallelizing the three test suites, we shrink the CI test to ~11-12 min in total https://github.com/Layr-Labs/eigenlayer-contracts/actions/runs/13209105775?pr=1074

  • unit test: 2-3 min
  • integration test: 9-10 min (total CI time is bounded by this test now)
  • fork test: 6-7 min (not exactly sure why this got 2x improvement down from 12-13 min, my theory is that when running in sequence previously, integration tests run before fork test, and integration tests may hv resources not shut down completely, leading to significantly slow execution of fork test)

Modifications:

parallelize unit, integration and fork tests to make CI time significantly faster

Result:

After your change, what will change.

0xClandestine and others added 30 commits January 3, 2025 15:17
* feat: add share helpers

* fix: add deposit scaling factor

* fix: rebase
* 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

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
* feat: initial deploy

* feat: slashing patch
* 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: add slashing docs
* chore: bindings
* docs: fixed commenting and updated queue withdrawal docs
* docs: minor cleanup

---------

Co-authored-by: Nadir Akhtar <[email protected]>
* fix: correct expected share calc

* chore: bindings

* fix: rounding on failing unit test
* 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
* fix: initialization params

* fix: roll blocks usage
* 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

* chore: forge fmt
* 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 delay bound

* test: remove unnecessary roll
* 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: burning

* chore: fmt

* chore: update storage report

* chore: update readme

* refactor: add burnableShares for epm storage

* chore: update storage report
* 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: 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]>
eigenmikem and others added 8 commits January 21, 2025 15:49
* 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
@bowenli86 bowenli86 requested review from 8sunyuan, ypatil12 and 0xClandestine and removed request for 8sunyuan, ypatil12 and 0xClandestine February 7, 2025 22:36
@bowenli86 bowenli86 force-pushed the fast branch 2 times, most recently from 0161e49 to 24e0236 Compare February 7, 2025 23:02
@ypatil12
Copy link
Collaborator

cc @0xClandestine did you do this in your latest CI update? Might be worth bringing to dev

@0xClandestine
Copy link
Member

cc @0xClandestine did you do this in your latest CI update? Might be worth bringing to dev

This is dope, let's create a new pr that targets dev @bowenli86 I can do it if you want. I had tried caching forge builds, but loading the cache is incredibly slow.

@bowenli86 bowenli86 changed the base branch from slashing-magnitudes-fixes to dev February 13, 2025 16:50
@bowenli86
Copy link
Member Author

closing, in favor of #1094

@bowenli86 bowenli86 closed this Feb 13, 2025
@bowenli86 bowenli86 deleted the fast branch February 13, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants