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

279 voting power is not synced properly on start of epoch #76

Merged

Conversation

Vitomir2
Copy link
Contributor

@Vitomir2 Vitomir2 commented Nov 12, 2024

Description

  • delete epoch.Validators.Copy() on creating NewValidatorSet in order to avoid redundant operations;
  • optimize if conditions for the end and start of epoch;
  • get the validators from the last epoch;
  • update the implementation for the generation of the syncValidatorsData input to properly extract the removed validators, too;
  • add a test for the function which includes added, updated and removed validators in the validators delta;
  • new testing createTestExtraForSyncValidatorsData function;

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

I have configured the blockchain locally and have tested if the syncValidatorsData's input is generated correctly and it seems all work fine. Additionally, added some tests to ensure that the extract function works properly.

@Vitomir2 Vitomir2 added the bug Something isn't working label Nov 12, 2024
@Vitomir2 Vitomir2 requested a review from R-Santev November 12, 2024 16:48
consensus/polybft/validator/validator_metadata.go Outdated Show resolved Hide resolved
consensus/polybft/validator/validator_metadata.go Outdated Show resolved Hide resolved
consensus/polybft/consensus_runtime_test.go Outdated Show resolved Hide resolved
consensus/polybft/consensus_runtime_test.go Outdated Show resolved Hide resolved
consensus/polybft/consensus_runtime_test.go Outdated Show resolved Hide resolved
@Vitomir2 Vitomir2 requested a review from R-Santev November 14, 2024 14:10
@Vitomir2 Vitomir2 changed the base branch from prod to develop November 15, 2024 08:00
use epoch.Validators.Copy() in order to ensure that the validators are not passed by reference and updated in some of the next functions;
optimize if conditions for the end and start of epoch;
get the validators from the last epoch;
update the implementation for the generation of the syncValidatorsData input to properly extract the removed validators, too;
delete the .Copy() as discussed with Roskata;
update the ExtractUpdatedValidatorsVotingPower logic to loop through the added, updated and then check removed validators;
avoid setting the VotingPower to 0 and add it as a parameter of the formatValidatorPower function;
create new test function createTestExtraForSyncValidatorsData and use it for the extraData;
make one test that includes added, updated and removed validators;
set concrete params instead of mock.Anything in the GetValidators mock;
@Vitomir2 Vitomir2 force-pushed the 279-voting-power-is-not-synced-properly-on-start-of-epoch branch from 1adf537 to 2e889e4 Compare November 15, 2024 08:25
@Vitomir2 Vitomir2 merged commit eb8a972 into develop Nov 15, 2024
5 checks passed
@Vitomir2 Vitomir2 deleted the 279-voting-power-is-not-synced-properly-on-start-of-epoch branch November 15, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants