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

Fix voting displayed percent #825

Merged
merged 5 commits into from
Apr 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion apps/voting/app/.babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,18 @@
"displayName": true
}],
"transform-runtime"
]
],
"env": {
"test": {
"presets": [
[
"env",
{
"modules": "commonjs",
"targets": { "node": "current" }
}
]
]
}
}
}
3 changes: 3 additions & 0 deletions apps/voting/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"devDependencies": {
"babel-core": "^6.26.0",
"babel-eslint": "^9.0.0",
"babel-jest": "^23.0.1",
"babel-plugin-styled-components": "^1.5.1",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-env": "^1.6.1",
Expand All @@ -37,11 +38,13 @@
"eslint-plugin-react": "^7.5.1",
"eslint-plugin-react-hooks": "^1.6.0",
"eslint-plugin-standard": "^4.0.0",
"jest": "^23.0.1",
"parcel-bundler": "1.9.7",
"prettier": "^1.8.2"
},
"scripts": {
"lint": "eslint ./src",
"test": "jest",
"sync-assets": "copy-aragon-ui-assets -n aragon-ui ./build && rsync -rtu ./public/ ./build",
"start": "npm run sync-assets && npm run watch:script & parcel serve index.html -p 3001 --out-dir build/",
"build": "npm run sync-assets && npm run build:script && parcel build index.html --out-dir build/ --public-url \".\"",
Expand Down
6 changes: 3 additions & 3 deletions apps/voting/app/src/components/VotingCard/VotingCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const VotingCard = React.memo(
align-items: center;
`}
>
{action ? <BadgeAction /> : <BadgeInformative />}
{action ? <BadgeAction /> : <BadgeQuestion />}
</div>
<Button compact mode="outline" onClick={handleOpen}>
View vote
Expand Down Expand Up @@ -121,9 +121,9 @@ VotingCard.defaultProps = {
onOpen: () => {},
}

const BadgeInformative = () => (
const BadgeQuestion = () => (
<Badge background="rgba(37, 49, 77, 0.16)" foreground="rgba(37, 49, 77, 1)">
Informative
Question
</Badge>
)

Expand Down
100 changes: 79 additions & 21 deletions apps/voting/app/src/math-utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import BN from 'bn.js'

/**
* Format numbers for a given number of decimal places
*
Expand Down Expand Up @@ -37,7 +39,7 @@ export function formatNumber(num, decimals = 2, { truncate = true } = {}) {
}

export function percentageList(values, digits = 0) {
return scaleValuesSet(values, digits)
return scaleBNValuesSet(values).map(value => value.toNumber())
}

/**
Expand All @@ -61,44 +63,100 @@ export function safeDiv(num, denom) {
return denom ? num / denom : 0
}

// Get a list of rounded 0 => `total` numbers, from a list of 0 => 1 values.
// If the total of the values is exactly 1, the total of the resulting values
// will be exactly `total`.
export function scaleValuesSet(values, digits = 0, total = 100) {
const digitsMultiplicator = Math.pow(10, digits)
// Scale to `total` a set of values summing to 1.
// Note that the accuracy of `values` is constrained to a set amount of decimals, as BN.js doesn't
// support decimal operations
export function scaleBNValuesSet(
values = [],
numTotal = 100,
correctionLimit = 0.001
AquiGorka marked this conversation as resolved.
Show resolved Hide resolved
) {
const total = new BN(numTotal)
const VALUES_ACCURACY_PLACES = 5
const VALUES_ACCURACY_ADJUSTER = Math.pow(10, VALUES_ACCURACY_PLACES)
const BN_VALUES_ACCURACY_ADJUSTER = new BN(VALUES_ACCURACY_ADJUSTER)

function highestValueIndex(values) {
return values
.map((value, index) => ({ value, index }))
.sort((v1, v2) => v2.value - v1.value)[0].index
}

if (values.length === 0) {
return []
}

let remaining = total * digitsMultiplicator
// Adjust values for accepted accuracy
values = values.map(
value =>
Math.floor(value * VALUES_ACCURACY_ADJUSTER) / VALUES_ACCURACY_ADJUSTER
)

const accumulatedTotal = values.reduce((total, v) => v + total, 0)
if (accumulatedTotal < 0) {
throw new Error('The sum of the values has to be a positive number.')
}
if (accumulatedTotal - correctionLimit > 1) {
throw new Error('The sum of the values has to be equal to or less than 1.')
}

// Get the difference to correct
const valuesCorrection = 1 - accumulatedTotal

const shouldCorrect =
valuesCorrection !== 0 &&
// Negative & out of limit have already thrown at this point,
// so we should correct if it’s below the correction limit.
valuesCorrection <= correctionLimit

// We always correct (up or down) the highest value
const correctionIndex = shouldCorrect ? highestValueIndex(values) : -1
if (correctionIndex > -1) {
values[correctionIndex] = values[correctionIndex] + valuesCorrection
}

// Track remaining so we can adjust later on
let remaining = total.clone()

// First pass, all numbers are rounded down
const percentages = values.map(value => {
const percentage = Math.floor(value * total * digitsMultiplicator)
remaining -= percentage
const scaledValues = values.map(value => {
const scaledValueAdjusted = total.mul(
new BN(value * VALUES_ACCURACY_ADJUSTER)
)
const scaledValue = scaledValueAdjusted.div(BN_VALUES_ACCURACY_ADJUSTER)

// Get the remaining amount in non-adjusted decimals
const remain =
scaledValueAdjusted.mod(BN_VALUES_ACCURACY_ADJUSTER).toNumber() /
VALUES_ACCURACY_ADJUSTER

remaining = remaining.sub(scaledValue)

return {
percentage,
remain: (value * total * digitsMultiplicator) % 1,
value,
scaledValue,
remain,
}
})

// Add the remaining to the value that is the closest
// to the next integer, until we reach `total`.
let index = -1
while (remaining--) {
index = percentages
.map(({ remain }, index) => ({ remain, index }))
.sort((p1, p2) => p2.remain - p1.remain)[0].index
while (remaining.gt(new BN(0))) {
index = highestValueIndex(scaledValues.map(({ remain }) => remain))

// The total of the values is not 1, we stop adjusting here
if (percentages[index].remain === 0) {
// The total of the values is not 1, we can stop adjusting here
if (scaledValues[index].remain === 0) {
break
}

percentages[index].percentage += 1
percentages[index].remain = 0
scaledValues[index].scaledValue = scaledValues[index].scaledValue.add(
new BN(1)
)
scaledValues[index].remain = 0

remaining = remaining.sub(new BN(1))
}

return percentages.map(p => p.percentage / digitsMultiplicator)
return scaledValues.map(p => p.scaledValue)
}
73 changes: 73 additions & 0 deletions apps/voting/app/src/math-utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import BN from 'bn.js'
import { scaleBNValuesSet } from './math-utils'

const scaleResults = (values, total, precision) =>
scaleBNValuesSet(values, new BN(total), precision).map(v => v.toString())

describe('scaleBNValuesSet()', () => {
const sets = [
[
'should add round up the first number first',
[[0.5, 0.5], '329879'],
['164940', '164939'],
],
[
'should work with very big numbers',
[[0.5, 0.5], '3298792983798273972398792837972310987189327'],
[
'1649396491899136986199396418986155493594664',
'1649396491899136986199396418986155493594663',
],
],
[
'should return an empty array if an empty array is provided',
[[], '100'],
[],
],
[
'should handle zero values',
[[0.9, 0, 0.1], '10000'],
['9000', '0', '1000'],
],
[
'should not correct the values if their sum is far enough from 1',
[[0.1, 0.1], '1000'],
['100', '100'],
],
[
'should correct the values if their sum is close enough to 1',
[[0.9, 0.099999], '1000'],
['900', '100'],
],
[
'should work with values that errored and made us debug this',
[[0.55, 0], '100'],
['55', '0']
]
]

sets.forEach(([label, params, results]) => {
test(label, () => {
expect(scaleResults(...params)).toEqual(results)
})
})

test('should throw if the numbers are too far from 1', () => {
expect(() => {
scaleResults([1.1, 0.3], '1000')
}).toThrow(Error)
expect(() => {
scaleResults([2, 5], '1000')
}).toThrow(Error)
})

test('should adapt to the correction limit', () => {
expect(scaleResults([0.8, 0.1], '1000')).toEqual(['800', '100'])
expect(scaleResults([0.9, 0.1001], '1000')).toEqual(['900', '100'])
expect(scaleResults([0.8, 0.1], '1000', 0.1)).toEqual(['900', '100'])
expect(scaleResults([0.9, 0.099], '1000', 0)).toEqual(['900', '99'])
expect(() => {
scaleResults([0.9, 1.0001], '1000', 0)
}).toThrow(Error)
})
})