Skip to content

feat: slashing audit fixes #475

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

Merged
merged 10 commits into from
May 7, 2025
Merged

feat: slashing audit fixes #475

merged 10 commits into from
May 7, 2025

Conversation

ypatil12
Copy link
Collaborator

@ypatil12 ypatil12 commented May 7, 2025

Motivation:

Middleware slashing audit fixes from Hexens and Dedaub.

Modifications:

Medium Sev

Low Sev

Informational/Docs

Result:

Audit fixes applied to slashing.

ypatil12 added 8 commits May 1, 2025 12:05
**Motivation:**

Per Hexens Eigen2-2, The current rate limit logic allows ejection beyond
the allowed limit.

**Modifications:**

Modify the ejection logic such that when the caller is the ejector, do
not automatically eject the first operator regardless of limit.

**Result:**

Proper rate limit assertions.
**Motivation:**

The `StakeRegistry` is missing a constraint check for modification of
lookahead time
of slashable quorum. This results in the Lookahead period potentially
being invalid.

**Modifications:**

Update `_setLookAheadPeriod` to revert if the `_lookaheadBlocks` is
greater than the `DEALLOCATION_DELAY`. Note that we'll also have to
update this check in `_createQuorum` since it does a `>=` check.
Operators are slashable for the entire delay, inclusive of the last
block.

**Result:**

Proper constraints.
NOTE: #464 was merged to wrong target

**Motivation:**

It is possible to break the `maxOperatorCount` invariant by doing the
following:

Let's assume there are two quorums, 1 and 2, with a `maxOperatorCount`
of 2.

1. Alice & Bob register for quorum 1
2. Bob registers for quorum 2
3. Bob deregisters from quorum 1, Charlie enters
4. Quorum 1 Members: Alice/Charlie. Quorum 2 members: Bob
5. Eve creates a churn registration that exits Bob. Quorum 1 has 3
members. This works just fine since we allow a churn to occur if the
`operatorToKick` is registered to the AVS (not the quorum):
https://github.com/Layr-Labs/eigenlayer-middleware/blob/f5adbcac55d9336cd646ce71bc467aa7e20f1a12/src/SlashingRegistryCoordinator.sol#L405-L408

Although this assumes that the `churnApprover` is buggy, we should still
be enforcing that you are churning a user if they are registered for the
quorum.

**Modifications:**

Require that the `operatorToKick` is registered for the quorum. 

**Result:**

Stricter churn guarantees.
**Motivation:**

An operator is slashable for at least `DEALLOCATION_DELAY`. The check
for the lookahead period should be exactly this amount, not less.

**Modifications:**

Update check for `lookAheadPeriod` to be `<=DEALLOCATION_DELAY`. 

**Result:**

Symmetric checks with #463
**Motivation:**

In `registerOperator` for the `NORMAL` registration type, we validate
the quorums' operator count twice, once within `_registerOperator`
(since we pass in `checkMaxOperatorCount: true`) and once after. This is
redundant.

**Modifications:**

Remove redundant check

**Result:**

Gas savings + cleaner code
**Motivation:**

`ServiceManagerBase` imported `LibMergeSort`

**Modifications:**

Remove unused import. 

**Result:**

Only used imports.
**Motivation:**

The stale stakes flag is buggy since it doesn't properly get the
`quorumUpdateBlockNumber` in the given range. The fix is to add
checkpointed storage for the latest quorum update block number. Since we
are moving towards a cert-verifier world, it's simpler to remove this
feature rather than ship checkpointed storage OR broken code.

**Modifications:**

Removed `staleStakesForbidden`. 

**Result:**

Cleaner code
**Motivation:**

Address informational

**Modifications:**

- Clearer modifier error:
9c73e50
- Remove unnecessary check:
0214b79
- Stale comments/Typos 

**Result:**

Cleaner Code
@ypatil12 ypatil12 requested a review from wadealexc May 7, 2025 22:51
@ypatil12 ypatil12 requested a review from pakim249CAL May 7, 2025 22:51
chore: fix naming
@ypatil12 ypatil12 force-pushed the feat/slashing-audit-fixes branch from e43b6a9 to bdc6eea Compare May 7, 2025 22:57
@ypatil12 ypatil12 merged commit fd26169 into dev May 7, 2025
4 of 5 checks passed
@ypatil12 ypatil12 deleted the feat/slashing-audit-fixes branch May 7, 2025 23:10
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.

2 participants