Skip to content

Pchain bls diffs fix cleanup #1594

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

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

  • Prevents duplicate iteration when fetching the primary network validator set
  • Uses the caching of the primary network validator set to avoid additional state iteration
  • Avoids unnecessary subnet validator set iteration
  • Avoids needing to handle the edge case when currentHeight == targetHeight for subnet BLS key tracking

How this works

Kept the majority of the code - just restructured it to be more efficient + have less edge cases.

How this was tested

CI

@StephenButtolph StephenButtolph self-assigned this Jun 7, 2023
@StephenButtolph StephenButtolph added the cleanup Code quality improvement label Jun 7, 2023
@StephenButtolph StephenButtolph added this to the v1.10.3 milestone Jun 7, 2023
Comment on lines +139 to +145
lastAcceptedHeight, err := m.GetCurrentHeight(ctx)
if err != nil {
return nil, err
}
if lastAcceptedHeight < height {
return nil, database.ErrNotFound
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kinda wish we didn't need to call this if the validator set was cached.... But this block should always be cached.

) error {
targetWeightDiffs, err := m.state.GetValidatorWeightDiffs(height, subnetID)
// Get the public keys for all the validators at [targetHeight]
primarySet, err := m.getValidatorSetFrom(currentHeight, targetHeight, constants.PrimaryNetworkID)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: we don't call makePrimaryNetworkValidatorSet because it may be cached. This does result in tracking another lookup in the metrics, but I feel like that's fine.

@abi87 abi87 merged commit 6b49006 into pchain_bls_diffs_fix Jun 8, 2023
@abi87 abi87 deleted the pchain_bls_diffs_fix_cleanup branch June 8, 2023 06:37
hexfusion pushed a commit to hexfusion/avalanchego that referenced this pull request Jun 22, 2023
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants