Skip to content

fix(protocol): fix _getOperatorForEpoch in PreconfWhitelist.sol #19329

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

Closed
wants to merge 13 commits into from

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Apr 24, 2025

Related to #19327.

@cyberhorsey I think your fix won't work -- you make the timestamp (for which the beacon root is queried) bigger, which makes the return value less predictable as the random seed. I think we should make the value smaller so we query a older beacon root as the random seed.

Please test both your PR and mine to verify.

@dantaik dantaik requested a review from cyberhorsey April 24, 2025 02:44
@dantaik dantaik marked this pull request as ready for review April 24, 2025 02:44
@dantaik dantaik changed the title Additional whitelist epoch fix fix(protocol): fix _getOperatorForEpoch in PreconfWhitelist.sol Apr 24, 2025
@cyberhorsey
Copy link
Contributor

cyberhorsey commented Apr 24, 2025

#19328

I deployed this one, and its working so far.

edit: nevermind, it changes the nextOperator mid epoch. I can try to deploy this one. If it doesnt work, we should just use the old getCurrent and getNext withotu this refactoring to have an internal fucntion for both.

Comment on lines +224 to +225
// selectorEpochOffset must be big enough to ensure a non-zero beacon root is
// available and immutable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does having big enough selectorEpochOffset ensure non-zero beacon root? In other words, isn't something like following enough?

Suggested change
// selectorEpochOffset must be big enough to ensure a non-zero beacon root is
// available and immutable.
// selectorEpochOffset must be big enough to ensure that the beacon root is finalized and
// cannot change due to reorg.

@dantaik dantaik marked this pull request as draft April 25, 2025 00:18
@dantaik dantaik closed this Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants