Skip to content
This repository was archived by the owner on Jun 11, 2024. It is now read-only.

Fix validator unshuffling #9088

Merged
merged 8 commits into from
Oct 13, 2023
Merged

Conversation

bobanm
Copy link
Contributor

@bobanm bobanm commented Oct 12, 2023

What was the problem?

This PR resolves #9040, resolves #9075 and resolves #9080

How was it solved?

  • Fixed the bug in the engine where the original validator list array was sorted and then saved in sorted order to BFT Params Store
  • Added config option [default to 0] to prevent validator shuffling before the configured block height, to ensure that the the new version can sync
  • BONUS: Cleaned up and simplified BFT Method unit tests 😎

How was it tested?

  • When running the app, validator's consecutive missed blocks now show correct value
  • When running the app, the validators are shuffled after each round
  • All tests are passing 👌🏻

@bobanm bobanm requested review from shuse2 and ishantiw October 12, 2023 14:01
@bobanm bobanm self-assigned this Oct 12, 2023
Copy link
Contributor

@ishantiw ishantiw left a comment

Choose a reason for hiding this comment

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

Apart for comments from @shuse2, I have a couple of minor comments, unit test is failing coz probably you need to update schema snapshot

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #9088 (26fa502) into release/6.0.0 (c9c1ecc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##           release/6.0.0    #9088   +/-   ##
==============================================
  Coverage          83.47%   83.47%           
==============================================
  Files                604      604           
  Lines              22803    22807    +4     
  Branches            3364     3365    +1     
==============================================
+ Hits               19035    19039    +4     
  Misses              3768     3768           
Files Coverage Δ
framework/src/engine/bft/method.ts 89.23% <100.00%> (+0.34%) ⬆️
framework/src/engine/bft/module.ts 100.00% <100.00%> (ø)
framework/src/engine/consensus/consensus.ts 82.68% <100.00%> (ø)
framework/src/engine/generator/generator.ts 81.88% <ø> (ø)
framework/src/schema/application_config_schema.ts 100.00% <ø> (ø)
framework/src/testing/fixtures/config.ts 41.66% <ø> (ø)
framework/src/types.ts 100.00% <ø> (ø)

Copy link
Collaborator

@shuse2 shuse2 left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@ishantiw ishantiw left a comment

Choose a reason for hiding this comment

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

👏

@shuse2 shuse2 merged commit 25e4ce7 into release/6.0.0 Oct 13, 2023
@shuse2 shuse2 deleted the 9040-fix-validator-unshuffling branch October 13, 2023 08:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants