Skip to content
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

Cleanup: unify typing of state variables in StakeRegistryStorage #127

Closed
wadealexc opened this issue Dec 29, 2023 · 4 comments · Fixed by #139
Closed

Cleanup: unify typing of state variables in StakeRegistryStorage #127

wadealexc opened this issue Dec 29, 2023 · 4 comments · Fixed by #139
Assignees

Comments

@wadealexc
Copy link
Collaborator

wadealexc commented Dec 29, 2023

/// @notice In order to register for a quorum i, an operator must have at least `minimumStakeForQuorum[i]`
/// evaluated by this contract's 'VoteWeigher' logic.
uint96[256] public minimumStakeForQuorum;
/// @notice array of the history of the total stakes for each quorum -- marked as internal since getTotalStakeFromIndex is a getter for this
StakeUpdate[][256] internal _totalStakeHistory;

This is the only place in the middleware contracts where we use [256] to represent quorum information. I'd like to replace this with a mapping for consistency. I don't think it actually impacts a single thing, but if it this ends up breaking things it's fine to keep as-is.

@wadealexc wadealexc self-assigned this Dec 29, 2023
@ChaoticWalrus
Copy link
Contributor

I think this suggestion is fine.
obviously it's a breaking storage change for upgradeability purposes, so now seems like the right time to make the change 😅

@wadealexc
Copy link
Collaborator Author

I think this suggestion is fine. obviously it's a breaking storage change for upgradeability purposes, so now seems like the right time to make the change 😅

Oh right, I didn't think about upgrading our Goerli contracts - would need to add some deprecated variables to make up the space.

@ChaoticWalrus
Copy link
Contributor

Oh right, I didn't think about upgrading our Goerli contracts - would need to add some deprecated variables to make up the space.

Aren't we already planning on redeploying because of other breaking storage layout / semantic changes? This strikes me as the last opportunity to reasonably make the change.

@wadealexc
Copy link
Collaborator Author

Oh right, I didn't think about upgrading our Goerli contracts - would need to add some deprecated variables to make up the space.

Aren't we already planning on redeploying because of other breaking storage layout / semantic changes? This strikes me as the last opportunity to reasonably make the change.

Will make sure I get a PR open for this today. I wanted to get the integration tests to a good state before I touched the contracts much more

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 a pull request may close this issue.

2 participants