-
Notifications
You must be signed in to change notification settings - Fork 114
fix: bls sig operator state retriever sorting #480
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
fix: bls sig operator state retriever sorting #480
Conversation
…or ids are ascending
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.
Minor comment, will approve after
import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; | ||
import {Math} from "@openzeppelin/contracts/utils/math/Math.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.
Nit: can we add a comment why we're copy-pasting these (due to not being on v5.1 OZ). Another option is to add another submodule
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.
addressed with 85ed41d
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.
LGTM. Will fix CI separately. Tests pass locally. This is an issue with fork PRs
Motivation:
Non-signer pubkeys were not properly ordered in the last iteration of this feature
Modifications:
Added OZ libraries as packed utilities for sorting, added sorting of the non-signers before returning them
Result:
Now all parameters returned are sorted in ascending order, which allows for strong assurances regarding non-duplicates in the array