Skip to content
This repository has been archived by the owner on Jun 17, 2020. It is now read-only.

Small improvements in DAO code #176

Merged
merged 4 commits into from Sep 4, 2018

Conversation

ManfredKarrer
Copy link
Member

No description provided.

- Remove check issuanceHeight <= txChainHeight as it is covered by
getWeightedMeritAmount method and throws an exception.
- We want to avoid division with double and possible rounding issues and
use long values instead. We get a resolution for 1 block with that
approach.

- Add tests and check for negative issuanceHeight
- Add sorting to BallotList
- Add comments
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

// We want a resolution of 1 block so we use the inverseAge and divide by maxAge afterwards to get the
// weighted amount
// We need to multiply first before we divide!
long weightedAmount = (amount * inverseAge) / maxAge;
Copy link
Member

Choose a reason for hiding this comment

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

I like this better than the previous calculations. Much easier to follow for the same effect.

@@ -569,10 +572,14 @@ private void applyParamChange(Set<EvaluatedProposal> acceptedEvaluatedProposals,
if (list.size() == 1) {
applyAcceptedChangeParamProposal((ChangeParamProposal) list.get(0).getProposal(), chainHeight);
} else if (list.size() > 1) {
log.warn("There have been multiple winning param change proposals with the same item. " +
"This is a sign of a social consensus failure. " +
"We treat all requests as failed in such a case.");
Copy link
Member

Choose a reason for hiding this comment

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

I think this is correct, with a social failure it's unsafe to continue. I don't think this is an attack vector since it needs more than 50% of votes attacking, which is already a bigger issue in itself.

@ManfredKarrer ManfredKarrer merged commit 4597613 into bisq-network:master Sep 4, 2018
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.

None yet

2 participants