feat: slashing magnitude poc #672
Closed
Conversation
This file contains 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
* feat: operator set scaffold * fix: impl/storage compile errors; pending updating of tests * chore: `forge fmt` * fix: `OperatorSet` struct misuse * fix: comment * chore: verbose use of `OperatorSet` * test: `registerOperatorToOperatorSet` * feat: `registerOperatorToOperatorSets` Enables registering multiple operator sets in a single call. * chore: `forge fmt` * feat: interface changes * fix: operator set digest * fix: `OPERATOR_SET_REGISTRATION_TYPEHASH` * chore: `forge fmt` * test: wrong avs using signature * fix: optimize for SSTOREs * test: `deregisterOperatorFromOperatorSets` * chore: rename `operatorSetStrategies` * test: `addStrategiesToOperatorSet` * test: `removeStrategiesFromOperatorSet` * test: more coverage * chore: improve natspec * WIP: simp mode * WIP: simp mode * WIP: simp mode - includes interface change, specifically the `StandbyParams` structure. `id` isn't needed for storage. * test: simp mode * test: simp mode * test: simp mode * refactor: simp mode storage * Revert "refactor: simp mode storage" This reverts commit 3b0450e. * Reapply "refactor: simp mode storage" This reverts commit 5f90d78. * feat: simp mode * fix(optimize): salt cancellation - remove check * test: improvements * test: improvements * fix: move `isOperatorSetAVS` update out of loop ooops * fix: standby update typehash * test: cleanup * fix: move mutation out of loop * nit: cleanup * fix: remove unused events --------- Co-authored-by: clandestine.eth <[email protected]>
chore: run
#636) * feat: bring back full storage; segregate events * feat: make operator set creation w/arrays; update interface * fix: update natspec * feat: update force dereg func * chore: fmt * feat: operator set migration (#637) * feat: add migration * test: add unit tests for migration * chore: format * fix: check operator set avs in parent func * fix: compilation * chore: format * feat: bring back opertor set struct (#639)
* feat: operator commission bips configured with a delay * build: bindings * fix: interfaces and comments * fix: storage gap and comments * chore: tests cleanup * build: bindings * chore: `forge fmt src/contracts` * feat: `operatorCommissionUpdates` length getter * fix: remove unused imports * fix: optimizations * chore: uncheckeds and remove dup view * build: bindings * chore: format --------- Co-authored-by: clandestine.eth <[email protected]>
also pushed updated bindings
* feat: track total members and sets * test: track total members and sets * fix: review changes * refactor: review changes * test: register multiple sets * test: deregister multiple sets * test: fuzz set creation
* feat: operator set rewards * chore: forge fmt * feat: add operator set retroactive length & genesis timestamp * docs: add deprecation note and fix typo * build: bindings * feat: add tests * chore: bindings --------- Co-authored-by: clandestine.eth <[email protected]> Co-authored-by: Michael Sun <[email protected]>
* fix: flakey tests * build: bindings * fix: out of gas error
gpsanant
reviewed
Aug 9, 2024
| function _allocate(address operator, MagnitudeAdjustment calldata allocation, uint32 effectTimestamp) internal { | ||
| IStrategy strategy = allocation.strategy; | ||
| OperatorSet[] calldata operatorSets = allocation.operatorSets; | ||
| uint64 freeAllocatableMagnitude = freeMagnitude[operator][strategy]; |
There was a problem hiding this comment.
prefer just freeMagnitude here
There was a problem hiding this comment.
same naming as mapping, dont want to underscore the mapping though because we want the built-in getter for it.
gpsanant
reviewed
Aug 9, 2024
| // 2. rate limiting of 1 pending allocation at a time | ||
| // read number of checkpoints after current timestamp | ||
| (uint224 value, uint256 pos) = _magnitudeUpdate[operator][strategy][operatorSets[i].avs][operatorSets[i] | ||
| .operatorSetId].upperLookupRecentWithPos(uint32(block.timestamp)); |
There was a problem hiding this comment.
does forge fmt yield this? gross
gpsanant
reviewed
Aug 9, 2024
| ); | ||
| // 2. rate limiting of 1 pending allocation at a time | ||
| // read number of checkpoints after current timestamp | ||
| (uint224 value, uint256 pos) = _magnitudeUpdate[operator][strategy][operatorSets[i].avs][operatorSets[i] |
There was a problem hiding this comment.
currentMagnitude224 or something? is there an issue with making the lib have a uint64 instead?
There was a problem hiding this comment.
Whatever unit we decide on I agree we should store that in the checkpoint struct to avoid redundant casting
gpsanant
reviewed
Aug 9, 2024
| require( | ||
| pos | ||
| == _magnitudeUpdate[operator][strategy][operatorSets[i].avs][operatorSets[i].operatorSetId].length() | ||
| - 1, |
There was a problem hiding this comment.
we gotta make it agnostic to num pending updates
gpsanant
reviewed
Aug 9, 2024
| .operatorSetId].upperLookupRecentWithPos(uint32(block.timestamp)); | ||
| // no checkpoint exists if value == 0 && pos == 0, so check the negation before checking if there is a | ||
| // a pending allocation | ||
| if (value != 0 || pos != 0) { |
There was a problem hiding this comment.
damn this is gross, but can't think of a better way haha.
There was a problem hiding this comment.
Maybe abstract this with a bool exists return type
gpsanant
reviewed
Aug 9, 2024
| if (value != 0 || pos != 0) { | ||
| require( | ||
| pos | ||
| == _magnitudeUpdate[operator][strategy][operatorSets[i].avs][operatorSets[i].operatorSetId].length() |
There was a problem hiding this comment.
prob want to move the -1 to a +1 on the other side to avoid underflow when we increase the number
gpsanant
reviewed
Aug 9, 2024
| ) internal { | ||
| IStrategy strategy = deallocation.strategy; | ||
| require( | ||
| deallocation.operatorSets.length == deallocation.magnitudeDiffs.length, |
There was a problem hiding this comment.
make this check in allocate too?
gpsanant
reviewed
Aug 9, 2024
| .upperLookupRecent(uint32(block.timestamp)) | ||
| ); | ||
| require( | ||
| deallocation.magnitudeDiffs[i] <= currentMagnitude, |
* feat: track sets operators in * feat: add natspec * refactor: reorganize * fix: tests passing * fix: compile warnings * fix: test passing * feat: add `operatorSetsMemberOf` pagination * test: add coverage * chore: bindings --------- Co-authored-by: Yash Patil <[email protected]>
gpsanant
reviewed
Aug 9, 2024
| uint64 slashedAmount = | ||
| uint64(uint256(bipsToSlash) * uint256(queuedDeallocation.magnitudeDiff) / BIPS_FACTOR); | ||
| queuedDeallocation.magnitudeDiff -= slashedAmount; | ||
| slashedFromDeallocation += slashedAmount; |
There was a problem hiding this comment.
Why not increment slashedMagnitude?
| OperatorSetRegistrationStatus memory registrationStatus = | ||
| operatorSetStatus[operatorSet.avs][operator][operatorSet.operatorSetId]; | ||
| return registrationStatus.registered | ||
| || registrationStatus.lastDeregisteredTimestamp + ALLOCATION_DELAY >= block.timestamp; |
gpsanant
reviewed
Aug 9, 2024
| function isOperatorSlashable(address operator, OperatorSet memory operatorSet) public view returns (bool) { | ||
| OperatorSetRegistrationStatus memory registrationStatus = | ||
| operatorSetStatus[operatorSet.avs][operator][operatorSet.operatorSetId]; | ||
| return registrationStatus.registered |
ypatil12
force-pushed
the
feat/operator-set-release
branch
from
cfca835 to
76f43d9
Compare
8sunyuan
force-pushed
the
slashing-magnitude-poc
branch
from
ef568e0 to
d795e1c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+434
to
+463
| function _registerToOperatorSets(address operator, address avs, uint32[] calldata operatorSetIds) internal { | ||
| // Loop over `operatorSetIds` array and register `operator` for each item. | ||
| for (uint256 i = 0; i < operatorSetIds.length; ++i) { | ||
| OperatorSet memory operatorSet = OperatorSet(avs, operatorSetIds[i]); | ||
|
|
||
| require( | ||
| isOperatorSet[avs][operatorSetIds[i]], | ||
| "AVSDirectory._registerOperatorToOperatorSets: invalid operator set" | ||
| ); | ||
|
|
||
| require( | ||
| !isMember(operator, operatorSet), | ||
| "AVSDirectory._registerOperatorToOperatorSets: operator already registered to operator set" | ||
| ); | ||
|
|
||
| ++operatorSetMemberCount[avs][operatorSetIds[i]]; | ||
|
|
||
| _operatorSetsMemberOf[operator].add(_encodeOperatorSet(operatorSet)); | ||
|
|
||
| OperatorSetRegistrationStatus storage registrationStatus = | ||
| operatorSetStatus[avs][operator][operatorSetIds[i]]; | ||
| require( | ||
| !registrationStatus.registered, | ||
| "AVSDirectory._registerOperatorToOperatorSets: operator already registered for operator set" | ||
| ); | ||
| registrationStatus.registered = true; | ||
|
|
||
| emit OperatorAddedToOperatorSet(operator, operatorSet); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
Comment on lines
+472
to
+488
| function _deregisterFromOperatorSets(address avs, address operator, uint32[] calldata operatorSetIds) internal { | ||
| // Loop over `operatorSetIds` array and deregister `operator` for each item. | ||
| for (uint256 i = 0; i < operatorSetIds.length; ++i) { | ||
| OperatorSet memory operatorSet = OperatorSet(avs, operatorSetIds[i]); | ||
|
|
||
| require( | ||
| isMember(operator, operatorSet), | ||
| "AVSDirectory._deregisterOperatorFromOperatorSet: operator not registered for operator set" | ||
| ); | ||
|
|
||
| --operatorSetMemberCount[avs][operatorSetIds[i]]; | ||
|
|
||
| _operatorSetsMemberOf[operator].remove(_encodeOperatorSet(operatorSet)); | ||
|
|
||
| emit OperatorRemovedFromOperatorSet(operator, operatorSet); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
* feat: implement pepe * move state into Storage contract * remove withdrawal proof method * feat: poc for partial withdrawal batching * feat: remove beaconChainOracle in favor of 4788 * modify verifyStaleBalance to use plural form * add pause flags for new methods * deprecate old state variables * minor cleanup and commenting * chore: get things compiling * i commented out/deleted a bajillion tests * fix: adjust storage footprint to be consistent with m2 * feat: adjust verifyStaleBalance to allow anyone to start a checkpoint * removes staleness concept from pod and manager state * clean: clean up start checkpoint logic * clean: remove comments * clean: remove outdated comment and rename proofs method * fix: remove unused variable and deprecate another * chore: rename lastFinalizedCheckpoint to lastCheckpointTimestamp * feat: add events for checkpoint creation and progression * feat: remove unneeded oracle interface from EigenPodManager * feat: remove unnecessary state root caching and add ValidatorWithdrawn event * feat: remove all use of the delayed withdrawal router (#524) * modify activateRestaking flow to use checkpointing * remove withdrawNonBeaconChainETHBalanceWei in favor of checkpointing * feat: remove staleness grace period * feat: add flag to startCheckpoint to prevent 0-balance checkpoints * chore: move currentCheckpoint to a public getter and update IEigenPod interface * chore: fix comment, update interfaces, add event * chore: clarify comment on activateRestaking * feat: skip validator if already checkpointed * fix: finish rebase * chore: make bindings * fix: swap inequality check to correctly skip duplicate proofs * chore: make bindings * test: modify integration test framework to support pepe (#563) * test: basic epoch processing * wip: balance proofs somewhat functional * test: flesh out beacon chain abi and test workflow * test: cleanup * test: add basic invariant checks for checkpoint proofs * test: add tests for full exits * feat: checkpoint proofs use balance container root * also refactors and cleans up BeaconChainProofs * more refactor/cleanup to come * chore: more proof library cleanup, removing unused constants * chore: additional cleanup and renaming of proof constants for consistency * chore: clean comments and reorganize constants * chore: remove delayedWithdrawalRouter from EigenPod * feat: adjust storage sizes for fields in checkpoint struct * feat: remove activateRestaking in favor of startCheckpoint (#577) * see PR comment for details * test: add proofgen test contract * fix: rename and add balance proof * feat: track balance exited for checkpoints * chore: deprecate deneb fork timestamp functions in EigenPodManager * test: fix existing integration tests * test: fix some unit tests and remove many outdated tests * test: start setting up new integration tests * fix: fixes two issues with verifyWC timing * verifyWC -> startCheckpoint in the same block no longer results in a bricked checkpoint * verifyWC using a timestamp older than the current checkpoint no longer allows you to submit a checkpoint proof for the new validator * chore: fix outdated comment * test: fleshed out eigenpod test flows * also reduced number of validators being generated by tests (for speed) * test: flesh out additional pod flows * chore: make bindings * test: add checks for several integration tests * fix: add additional pause condition for verifyStaleBalance * docs: add initial EigenPod docs * docs: clean and update EigenPodManager docs * chore: small wip to eigenpod docs and contract comment cleanup * chore: fix gas metering test to be consistent * also minor clarity tweak in verifyCheckpointProofs * test: eigenpod unit tests with checkpointing (#591) * test: testings init * test: eigenpod unit tests refactor * test: startCheckpoint unit tests * test: pod unit tests * fix: rebase changes * chore: make bindings * chore: revert pod changes * test: add several tests and checks --------- Co-authored-by: wadealexc <[email protected]> * chore: cleanup dwr and unused code (#593) * chore: cleanup dwr and unused code * chore: comment out pod specs * feat: remove staleness timing window * chore: update IEigenPod interface with updated comments * chore: fix bindings * test: finish verify start complete flow for pepe integration tests * chore: fix bindings * test: add slashing and native eth integration tests * build: partial withdrawal batching upgrade scripts (#598) * build: preprod pod upgrade scripts * chore: cleanup unused files * chore: add pepe deployment output * docs: finish main eigenpod docs and improve commenting * docs: finish main eigenpod docs * feat: remove hasRestaked and lastCheckpointTimestamp checks * test: add tests for constructor and initialize * test: fix mainnet fork tests and compiler warnings * docs: update diagrams for pepe * chore: upgrade preprod eigenpods (#611) * chore: upgrade preprod eigenpods * chore: remove unneeded logs * chore: deploy and update deployment addresses * feat: public block root getter (#612) * docs: update user flow diagrams to mention supported tokens * also increases resolution * feat: add proof submitter address (#629) * feat: add proof submitter address * test: add event emission test * docs: fix comments and add proof submitter to docs * chore: add sigma prime audit * feat: deploy new pods to holesky preprod * includes proofSubmitter address * feat: update PEPE events (#632) * feat: mock out new events for EigenPodManager * chore: make bindings * feat: remove unneeded event change and update tests * chore: make bindings * fix: final event versions * chore: upgrade preprod with new PEPE events * docs: update audit report * fix: reject credential proofs if activation epoch is not set (#668) * fix: reject credential proofs if activation epoch is not set * chore: make bindings * chore: fix formatting and borked config * fix: fix borked addresses in holesky config * chore: fix formatting again * chore: upgrade preprod with new credential check * chore: deploy pepe to holesky
* feat: operator set scaffold * fix: impl/storage compile errors; pending updating of tests * chore: `forge fmt` * fix: `OperatorSet` struct misuse * fix: comment * chore: verbose use of `OperatorSet` * test: `registerOperatorToOperatorSet` * feat: `registerOperatorToOperatorSets` Enables registering multiple operator sets in a single call. * chore: `forge fmt` * feat: interface changes * fix: operator set digest * fix: `OPERATOR_SET_REGISTRATION_TYPEHASH` * chore: `forge fmt` * test: wrong avs using signature * fix: optimize for SSTOREs * test: `deregisterOperatorFromOperatorSets` * chore: rename `operatorSetStrategies` * test: `addStrategiesToOperatorSet` * test: `removeStrategiesFromOperatorSet` * test: more coverage * chore: improve natspec * WIP: simp mode * WIP: simp mode * WIP: simp mode - includes interface change, specifically the `StandbyParams` structure. `id` isn't needed for storage. * test: simp mode * test: simp mode * test: simp mode * refactor: simp mode storage * Revert "refactor: simp mode storage" This reverts commit 3b0450e. * Reapply "refactor: simp mode storage" This reverts commit 5f90d78. * feat: simp mode * fix(optimize): salt cancellation - remove check * test: improvements * test: improvements * fix: move `isOperatorSetAVS` update out of loop ooops * fix: standby update typehash * test: cleanup * fix: move mutation out of loop * nit: cleanup * fix: remove unused events --------- Co-authored-by: clandestine.eth <[email protected]>
* feat: operator commission bips configured with a delay * build: bindings * fix: interfaces and comments * fix: storage gap and comments * chore: tests cleanup * build: bindings * chore: `forge fmt src/contracts` * feat: `operatorCommissionUpdates` length getter * fix: remove unused imports * fix: optimizations * chore: uncheckeds and remove dup view * build: bindings * chore: format --------- Co-authored-by: clandestine.eth <[email protected]>
* feat: operator set rewards * chore: forge fmt * feat: add operator set retroactive length & genesis timestamp * docs: add deprecation note and fix typo * build: bindings * feat: add tests * chore: bindings --------- Co-authored-by: clandestine.eth <[email protected]> Co-authored-by: Michael Sun <[email protected]>
* fix: flakey tests * build: bindings * fix: out of gas error
|
Alright, this branch is completely bricked and rebase process is all funky. Going to just migrate the 2 commits from this branch into another PR again |
|
Updated PR here #679 |
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.
Updated to original interface. TL;DR
freeMagnitudein storageslashOperator, slashes current magnitude as well as queued deallocations.TODOs:
Several TODOs commented out in the code as well
Deposits/Withdraws of Shares [NOT STARTED]
Allocator Configuration [WIP]
OperatorDetails.earningsReceiver?queueDeallocation(),slashOperator()