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

Refactor assignment container to include proposer slots #14038

Closed
wants to merge 3 commits into from

Conversation

terencechain
Copy link
Member

No functional changes. This PR refactors

  • Adds ProposerSlots fields to CommitteeAssignmentContainer
  • CommitteeAssignments return one less argument since proposer slots is already part of the container
  • A few renaming for clarity

@terencechain terencechain added Ready For Review A pull request ready for code review Cleanup Code health! labels May 22, 2024
@terencechain terencechain requested a review from a team as a code owner May 22, 2024 14:27
@terencechain terencechain requested review from nalepae, potuz and nisdas May 22, 2024 14:27
if err != nil {
return nil, err
}
assignmentMap := make(map[primitives.ValidatorIndex]*AssignmentContainer, len(activeValidatorIndices))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why such a large allocation?

Comment on lines +196 to +197
// If previous proposer indices computation is outside if current proposal epoch range,
// we need to reset state slot back to start slot so that we can compute the correct committees.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small grammar fixes

Suggested change
// If previous proposer indices computation is outside if current proposal epoch range,
// we need to reset state slot back to start slot so that we can compute the correct committees.
// If previous proposer indices computation is outside of the current proposal epoch range,
// we need to reset the state slot back to the start slot so that we can compute the correct committees.

@terencechain
Copy link
Member Author

Deprecated by #14039

@terencechain terencechain deleted the assignment-container branch June 5, 2024 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code health! Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants