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

Improve data structure, validation and performance #2556

merged 55 commits into from Mar 23, 2019


None yet
3 participants
Copy link

commented Mar 18, 2019

Fixes #2554
Fixes #2555
Fixes #2545
Fixes #2539
Fixes #2415

Adding new params and bonded roles is possible after adding the UNDEFINED enums and avoid throwing exceptions if enum is not found.

ManfredKarrer added some commits Mar 18, 2019

Add UNDEFINED enum entry to all dao enums to allow updates
For backward compatibility we resolve an unknown new enum entry with
UNDEFINED if UNDEFINED is available in the enum.
Adding a bonded role and adding a param is tested with that and works.
Other changes need to be tested before implemented as they might have
more consequences for consensus.

@ManfredKarrer ManfredKarrer added this to the v0.9.6 milestone Mar 18, 2019

ManfredKarrer added some commits Mar 18, 2019

The price node will likely be used for further data relay tasks and the
delivery of the fee is already not well convered by the price term...
Use ProposalValidatorProvider to operate on concrete ProposalValidator
We used ProposalValidator for most validation processes but that missed
the custom validation in the sub classed for each proposal type.
ProposalValidator is now abstract and ProposalValidatorProvider returns
instance matching to proposal type.
Use block height of proposal tx if available
To be able to apply the validation also on past cycles we need to use
the block height if the proposal tx and not the current one. Just in
case the tx is not confirmed (when temp proposal gets published) we use
the current height, but that would anyway match the cycle.
In a past cycle params could have been different and validation need to
use the correct param value from that cycle.

@ManfredKarrer ManfredKarrer requested review from sqrrm and devinbileck Mar 19, 2019

ManfredKarrer added some commits Mar 19, 2019

Change blind vote phase and vote reveal durations
Blind vote phase changed from 4 days to 3 days and vote reveal from
2 days to 3 days.
Remove requiredQuorum and requiredThreshold from EvaluatedProposal
We would break the hash chain if we would change the quorum or
threshold values in the param enum.

ManfredKarrer added some commits Mar 21, 2019

Remove SHA3-256 hash as it turned out it is slower as SHA256
In some tests it seemed that SHA3-256 is 30% fast than SHA256 but later
with the data we used in dao testnet it was actually slower.
To avoid confusion which hash function to use and to avoid mixing them
without strong reason I prefer to remove it again.
Update json export for vote results
Try to reflect domain structure and field names.
Add BONDED_ROLE_FACTOR to param to react on BSQ price changes
To avoid the need to change the required bond in the BondedRoleType
if the BSQ price changes we use the BONDED_ROLE_FACTOR param where the
factor can be changed. In the BondedRoleType we use the requiredBondUnit
which will be multiplied with the BONDED_ROLE_FACTOR value to get the
required bond amount.
Remove nonBsqTxOutputMap from daoState
We can evaluate the nonBsqTxOutputs without storing it.
Storing them in the map would have required to remove them as well
once withdrawn from the wallet.

@ManfredKarrer ManfredKarrer marked this pull request as ready for review Mar 22, 2019

@ManfredKarrer ManfredKarrer requested a review from ripcurlx as a code owner Mar 22, 2019


This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2019

@sqrrm @devinbileck @ripcurlx
I know that is a monster PR and hard to review.
Still would love to get it soon merged so we have some time for dev testing before next release.
I will continue testing the next hours, but tested already quite a lot. If nobody can review in the next hour I will consider to merge despite an ACK. If any NACK arrives I will not merge of course....

@ManfredKarrer ManfredKarrer changed the title Misc dao improvements Improve data structure, validation and performance Mar 22, 2019

Copy link

left a comment

Overall looks fine to me. However, I am not well experienced in this area of the code and can just make general observations.
Only concern I had was regarding translations for the validation messages. I assume that is still todo and will be completed separately?


This comment has been minimized.

Copy link
Member Author

commented Mar 22, 2019

Yes that is on my todo list. I try to focus on the critical issue first and keep that for later.

ManfredKarrer added some commits Mar 22, 2019

Add missing @EqualsAndHashCode(exclude = {"date"})
Was unintendely removed in latest refactorings

@ManfredKarrer ManfredKarrer merged commit 47fb177 into bisq-network:master Mar 23, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed

@ManfredKarrer ManfredKarrer deleted the ManfredKarrer:misc-dao-improvements branch Mar 23, 2019

Copy link

left a comment


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.