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

Dao fix majorityvote issue #2371

Merged

Conversation

ManfredKarrer
Copy link
Contributor

Fixes #2362

We need to add any valid vote reveal tx / blind vote tx pair for the
majority hash calculation even if the blind vote payload is missing as
that could be relevant for the majority hash calculation. We add an
empty ballotList and meritList in such cases.
@ManfredKarrer ManfredKarrer requested a review from sqrrm February 5, 2019 13:26
@ManfredKarrer ManfredKarrer changed the title Dao fix majorityvote issue [WIP] Dao fix majorityvote issue Feb 5, 2019
@ManfredKarrer
Copy link
Contributor Author

Needs more testing before merge.

We had 2 times the onParseTxsComplete called in the version before
To avoid empty handlers I have set all methods to default so the client
code implements only the used one.
Move handler code from onNewBlockHeight to
onParseTxsCompleteAfterBatchProcessing to avoid too much UI updates
while parsing.
@ManfredKarrer ManfredKarrer changed the title [WIP] Dao fix majorityvote issue Dao fix majorityvote issue Feb 6, 2019
@ManfredKarrer
Copy link
Contributor Author

@sqrrm I have tested now with different scenarios with blind vote missing, ignore merit for majorit hash case,... Could you give it a critical review and test cycle as well?

# Conflicts:
#	desktop/src/main/java/bisq/desktop/main/dao/governance/make/MakeProposalView.java
@sqrrm
Copy link
Member

sqrrm commented Feb 7, 2019

I am looking at this but it will take a while as it's rather complicated. @ripcurlx hope to be done before the next release as I think this is quite important to get in.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

I added some refactoring as a PR to manfred's repo. Had to refactor to understand what's going on. I haven't tested this yet.

@ManfredKarrer
Copy link
Contributor Author

@sqrrm I will merge to not delay release, but please go on testing, its a very critical PR.

@ManfredKarrer ManfredKarrer merged commit 12d2879 into bisq-network:master Feb 9, 2019
@ManfredKarrer ManfredKarrer deleted the dao-fix-majorityvote-issue branch February 14, 2019 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants