-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: remove is_vote_valid #5363
Conversation
721d3c4
to
685ca04
Compare
0cb4bb4
to
4cd5a1e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5363 +/- ##
======================================
Coverage 71% 71%
======================================
Files 495 497 +2
Lines 86434 86199 -235
Branches 86434 86199 -235
======================================
- Hits 61703 61544 -159
+ Misses 21968 21876 -92
- Partials 2763 2779 +16 ☔ View full report in Codecov by Sentry. |
state-chain/pallets/cf-elections/src/electoral_systems/tests/monotonic_change.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_vote_needed is currently this:
match current_vote {
AuthorityVoteOf::<Self>::Vote(current_vote) => current_vote != proposed_vote,
_ => false,
}
Should it not be:
match current_vote {
AuthorityVoteOf::<Self>::Vote(current_vote) => current_vote.value != proposed_vote.value,
_ => false,
}
ie. we only want a cote if the value has changed, not if the block has changed?
Otherwise the engines will just vote almost continuously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good point, better that way
* fix: correct shared data hash * fix: remove is_vote_valid * test: fix tests * chore: remove stale comment * fix: only compare value, not slot number * chore: clippy
* fix: correct shared data hash * fix: remove is_vote_valid * test: fix tests * chore: remove stale comment * fix: only compare value, not slot number * chore: clippy
…d-migration * origin/main: fix: retry with search history enabled after some attempts (#5370) fix: rpc subscription return types (#5374) chore: remove unused stream_utils (#5373) fix: always sweep on RPC free balance (#5376) fix: make boost detection more accurate (#5369) chore: better not contributing error message (#5368) chore: rename ElectoralSystemStatus -> ElectionPalletStatus (#5367) fix: `request_swap_deposit_address_with_affiliates` refund address is consistent with destination address type (#5360) feat: add sol chain tracking metric (#5366) fix: remove is_vote_valid (#5363) fix: separate lp account for lp api (#5354) fix: features for test-externalities (#5365) Update localnet polkadot runtime to v1.3.3 (#5337)
Pull Request
Closes: PRO-1757
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
is_vote_valid
was only used once, and was used erroneously. Plus, if a single vote within a group is invalid then the whole extrinsic vails, effectively invalidating all the votes. We have is_vote_needed to prevent engines from submitting too many votes. So have removed it, which simplifies things too. No need to hash the data at all, we can compare the values directly in check_consensus.