Fix voting displayed percent (#825)

* VotingCard: update informative to question

* Math utils: refactor scale values set to use bn

* Add jest and babel-jest deps for tests

* babelrc: add env tests presets

* Add math utils tests

apps/voting/app/.babelrc

@@ -23,5 +23,18 @@
       "displayName": true
     }],
     "transform-runtime"
-  ]
+  ],
+  "env": {
+    "test": {
+      "presets": [
+        [
+          "env",
+          {
+            "modules": "commonjs",
+            "targets": { "node": "current" }
+          }
+        ]
+      ]
+    }
+  }
 }

apps/voting/app/package.json

@@ -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",
@@ -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 \".\"",

apps/voting/app/src/components/VotingCard/VotingCard.js

@@ -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
@@ -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>
 )
 

apps/voting/app/src/math-utils.js

@@ -1,3 +1,5 @@
+import BN from 'bn.js'
+
 /**
  * Format numbers for a given number of decimal places
  *
@@ -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())
 }
 
 /**
@@ -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
+) {
+  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)
 }

apps/voting/app/src/math-utils.test.js

@@ -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)
+  })
+})