-
Notifications
You must be signed in to change notification settings - Fork 331
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
added read functions for eigenpods #338
Conversation
src/contracts/pods/EigenPod.sol
Outdated
require(validatorPubkey.length == 48, "EigenPod.validatorPubkeyHashToInfo must be a 48-byte BLS public key"); | ||
bytes32 validatorPubkeyHash = _calculateValidatorPubkeyHash(validatorPubkey); |
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.
this is a duplicate check -- you have the same require
statement in the _calculateValidatorPubkeyHash
function. I think it's reasonable to delete this first check (since you also use the length check in validatorPubkeyToInfo
, you should not delete the check in _calculateValidatorPubkeyHash
)
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.
fixed!
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.
These seem like great additions!
I believe this PR should be targeting the m2-mainnet
branch instead of master
-- change that + address the one small nit and I'm cool with merging.
Looks like Im gonna have to make a new PR. I will do that and request your review again |
closing this out. the other PR mentioned above was already created + merged. |
Making this PR based on requests to access these functions. Since the pubkey hash is not simply sha256(pubkey) but rather sha256(pubkey + bytes(16)), adding these would make it easier for users to call these functions with data they have on hand. Specifically, Figment requested this.
Note: maybe worht just deleting the other functions that take in the pubkey hash if we dont use them actively for the FE?