fix: remove token param from Deposit event and related APIs #1013

bowenli86 · 2025-01-10T22:29:05Z

fix: remove token param from Deposit event and related APIs, to remove redundancy and as a fix to the reported bug of token param is incorrectly populated

ypatil12

LGTM, needs 1 more approval

8sunyuan · 2025-01-13T15:30:47Z

I'm assuming no impact to EigenStrategy since that can either take a deposit of EIGEN or bEIGEN. If so then lgtm

Need to evaluate EigenStrat

ypatil12 · 2025-01-13T16:13:39Z

I'm assuming no impact to EigenStrategy since that can either take a deposit of EIGEN or bEIGEN. If so then lgtm

We should be good here in terms of mechanics.

On deposits -> SM calls strategyBase, no interaction with DM or relevant interfaces removed.

On withdrawals via shares, we simply add shares back, bEIGEN strategy doesn't care here.

There are two drawbacks though:

You cannot answer the question: are people depositing EIGEN or bEIGEN into the eigen strategy
In a multi-token strat world, we would have re-add the token parameter

bowenli86 · 2025-01-13T17:51:27Z

I'm assuming no impact to EigenStrategy since that can either take a deposit of EIGEN or bEIGEN. If so then lgtm

We should be good here in terms of mechanics.

On deposits -> SM calls strategyBase, no interaction with DM or relevant interfaces removed.

On withdrawals via shares, we simply add shares back, bEIGEN strategy doesn't care here.

There are two drawbacks though:

You cannot answer the question: are people depositing EIGEN or bEIGEN into the eigen strategy

In a multi-token strat world, we would have re-add the token parameter

chatted w/ Yash

EIGEN<>bEIGEN conversion is better tracked separately b/c there are multiple entry points to convert them (Deposit, Withdraw, or just contract calls), and we can associate transaction ids to the source entry points. We'd either 1/ add Wrapped/Unwrapped events to Eigen.sol (tbd w/ jeff) as preferred way, or 2/ use Transfer event in _mint/_burn which tho has too many transfers and is noisy

we don't need to worry about multi-token strat for now

bowenli86 · 2025-01-13T18:59:18Z

synced w/ Jeff, we'll add Wrapped/Unwrapped events to Eigen.sol as part of an upcoming ELIP

wadealexc

Needs make bindings

and updates to documentation:

* 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

@8sunyuan

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 feat: slashing patch upgrade script (#967) * feat: initial deploy * feat: slashing patch fix non-present upgrade.json 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 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]> 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]> 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 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]> test: enable shared setups for integration tests (#1036) * 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 (#1013) * 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: changing burnableShares to EnumerableMap (#1028) * 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]> feat: add `getAllocatedStake` and update `getMinimumSlashableStake` (#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]> test(integration): implement registration and allocation invariants (#1042) * chore: fix forge nightly release breaking two tests * test: fix outdated alm tests fix: delegate shares (#1045) **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]> fix: signature utils (#1015) - *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. feat: require avs register metadata in allocation manager (#1025) require avs register metadata in allocation manager before they can create operatorset --------- Co-authored-by: clandestine.eth <[email protected]>

bowenli86 requested review from 0xClandestine and ypatil12 and removed request for 0xClandestine January 10, 2025 23:29

ypatil12 previously approved these changes Jan 13, 2025

View reviewed changes

ypatil12 requested a review from 8sunyuan January 13, 2025 14:31

8sunyuan approved these changes Jan 13, 2025

View reviewed changes

bowenli86 force-pushed the removetoken branch from 01abd19 to 402003c Compare January 13, 2025 17:44

wadealexc force-pushed the slashing-magnitudes-fixes branch from 1427a85 to 403e0a1 Compare January 13, 2025 19:58

bowenli86 added 7 commits January 16, 2025 11:15

fix: remove token param from Deposit event and related APIs

ea2276b

fix: forge fmt

c167d45

fix: rebase

0415f39

fix: update EigenPodManager and test

9c96260

fix: update tests

8542d65

fix: update eigenpodmanager tests

7d2fe2b

fix: update StrategyManagerMock

5f24617

bowenli86 force-pushed the removetoken branch from 402003c to 5f24617 Compare January 16, 2025 19:16

wadealexc suggested changes Jan 16, 2025

View reviewed changes

bowenli86 added 2 commits January 16, 2025 12:11

feat: add bindings

b136343

fix: update docs

8ec3896

bowenli86 requested review from wadealexc and ypatil12 January 17, 2025 03:53

wadealexc approved these changes Jan 17, 2025

View reviewed changes

Merge branch 'slashing-magnitudes-fixes' into removetoken

31d7e3e

bowenli86 merged commit 5cfc840 into slashing-magnitudes-fixes Jan 17, 2025
11 checks passed

bowenli86 deleted the removetoken branch January 17, 2025 19:04

wadealexc mentioned this pull request Feb 18, 2025

feat: slashing audit fixes #1046

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: remove token param from Deposit event and related APIs #1013

fix: remove token param from Deposit event and related APIs #1013

Uh oh!

bowenli86 commented Jan 10, 2025 •

edited

Loading

Uh oh!

ypatil12 left a comment

Uh oh!

8sunyuan commented Jan 13, 2025 •

edited

Loading

Uh oh!

ypatil12 commented Jan 13, 2025

Uh oh!

bowenli86 commented Jan 13, 2025 •

edited

Loading

Uh oh!

bowenli86 commented Jan 13, 2025

Uh oh!

wadealexc left a comment

Uh oh!

Uh oh!

Uh oh!

fix: remove token param from Deposit event and related APIs #1013

fix: remove token param from Deposit event and related APIs #1013

Uh oh!

Conversation

bowenli86 commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ypatil12 left a comment

Choose a reason for hiding this comment

Uh oh!

8sunyuan commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ypatil12 commented Jan 13, 2025

Uh oh!

bowenli86 commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bowenli86 commented Jan 13, 2025

Uh oh!

wadealexc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bowenli86 commented Jan 10, 2025 •

edited

Loading

8sunyuan commented Jan 13, 2025 •

edited

Loading

bowenli86 commented Jan 13, 2025 •

edited

Loading