-
Notifications
You must be signed in to change notification settings - Fork 114
fix: multichain pt1 audit fixes #520
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
Conversation
2b51e9b
to
f32dc54
Compare
**Motivation:** Middleware contains tons of unused imports, which reduces code clarity. **Modifications:** - Used `forge lint src --only-lint unused-import` to remove all unused imports from `src`. **Result:** Improved clarity.
**Motivation:** We now have two `ISocketRegistry` interfaces, and need to differentiate for clarity. **Modifications:** - Renamed **new** `ISocketRegistry` -> `ISocketRegistryV2` **Result:** Improved clarity.
**Motivation:** If the `weights` array has a different length or stake composition, cert verification would silently fail. There is no way to enforce this for BN254. To opt for similar behavior, we clarify this in our natspec. **Modifications:** Update natspec/docs on risks of different array lengths. **Result:** Clearer code --------- Co-authored-by: Nadir Akhtar <nadir-akhtar@users.noreply.github.com>
**Motivation:** Update incorrect natspec **Modifications:** - Clarify `KeyRegistrar` usage - Clarify `calculateOperatorTable` usage **Result:** Clearer natspec
**Motivation:** The following natspec is present but isn't true and suggest improper use. ```solidity /// @dev The immutable avs address `AVSRegistrar` is NOT the address of the AVS in EigenLayer core. /// @dev The address of the AVS in EigenLayer core is the proxy contract, and it is set via the `initialize` function below. ``` **Modifications:** - Removed confusing comments - Made AVS var stateful and `AVSRegistrar` abstract **Result:** Improved clarity.
ec35d75
to
f3580bd
Compare
import {IAVSRegistrar} from "eigenlayer-contracts/src/contracts/interfaces/IAVSRegistrar.sol"; | ||
import {IAVSRegistrarInternal} from "./IAVSRegistrarInternal.sol"; | ||
import {ISocketRegistry} from "./ISocketRegistryV2.sol"; | ||
import {ISocketRegistryV2} from "./ISocketRegistryV2.sol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the latest interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
chore: update test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the latest changes from main
on eigenlayer-contracts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest changes from the corresponding contracts PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should that be merged first before this is merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fine since that will be latest commit (soon)
4db21e4
to
a6806d4
Compare
Multichain pt1. Audit Fixes:
High:
Informational:
ISocketRegistry
->ISocketRegistryV2
#511weights
array structure #510