Move safeNumber to shared util and tweak implementation

packages/loot-core/src/server/budget/actions.js

@@ -3,7 +3,7 @@ import * as db from '../db';
 import * as prefs from '../prefs';
 import * as sheet from '../sheet';
 import { batchMessages } from '../sync';
-import { safeNumber } from './util';
+import { safeNumber } from '../../shared/util';
 
 async function getSheetValue(sheetName, cell) {
   const node = await sheet.getCell(sheetName, cell);

packages/loot-core/src/server/budget/report.js

@@ -1,3 +1,4 @@
+import { safeNumber } from '../../shared/util';
 import * as sheet from '../sheet';
 import { number, sumAmounts } from './util';
 
@@ -25,17 +26,17 @@ export async function createCategory(cat, sheetName, prevSheetName) {
     ],
     run: (budgeted, sumAmount, prevCarryover, prevLeftover) => {
       if (cat.is_income) {
-        return (
+        return safeNumber(
           number(budgeted) -
-          number(sumAmount) +
+            number(sumAmount) +
-          (prevCarryover ? number(prevLeftover) : 0)
+            (prevCarryover ? number(prevLeftover) : 0)
         );
       }
 
-      return (
+      return safeNumber(
         number(budgeted) +
-        number(sumAmount) +
+          number(sumAmount) +
-        (prevCarryover ? number(prevLeftover) : 0)
+          (prevCarryover ? number(prevLeftover) : 0)
       );
     }
   });
@@ -50,7 +51,7 @@ export async function createCategory(cat, sheetName, prevSheetName) {
     refresh: true,
     run: (budgeted, sumAmount, carryover) => {
       return carryover
-        ? Math.max(0, number(budgeted) + number(sumAmount))
+        ? Math.max(0, safeNumber(number(budgeted) + number(sumAmount)))
         : sumAmount;
     }
   });
@@ -109,7 +110,7 @@ export function createSummary(groups, categories, sheetName) {
     initialValue: 0,
     dependencies: ['total-income', 'total-spent'],
     run: (income, spent) => {
-      return income - -spent;
+      return safeNumber(income - -spent);
     }
   });
 }

packages/loot-core/src/server/budget/rollover.js

@@ -1,6 +1,7 @@
 import * as monthUtils from '../../shared/months';
 import * as sheet from '../sheet';
-import { number, sumAmounts, flatten2, unflatten2, safeNumber } from './util';
+import { number, sumAmounts, flatten2, unflatten2 } from './util';
+import { safeNumber } from './util';
 
 const { resolveName } = require('../spreadsheet/util');
 
@@ -51,10 +52,10 @@ export function createCategory(cat, sheetName, prevSheetName) {
         `${prevSheetName}!leftover-pos-${cat.id}`
       ],
       run: (budgeted, spent, prevCarryover, prevLeftover, prevLeftoverPos) => {
-        return (
+        return safeNumber(
           number(budgeted) +
-          number(spent) +
+            number(spent) +
-          (prevCarryover ? number(prevLeftover) : number(prevLeftoverPos))
+            (prevCarryover ? number(prevLeftover) : number(prevLeftoverPos))
         );
       }
     });
@@ -78,7 +79,7 @@ export function createSummary(groups, categories, prevSheetName, sheetName) {
   sheet.get().createDynamic(sheetName, 'from-last-month', {
     initialValue: 0,
     dependencies: [`${prevSheetName}!to-budget`, `${prevSheetName}!buffered`],
-    run: (toBudget, buffered) => number(toBudget) + number(buffered)
+    run: (toBudget, buffered) => safeNumber(number(toBudget) + number(buffered))
   });
 
   // Alias the group income total to `total-income`
@@ -91,7 +92,8 @@ export function createSummary(groups, categories, prevSheetName, sheetName) {
   sheet.get().createDynamic(sheetName, 'available-funds', {
     initialValue: 0,
     dependencies: ['total-income', 'from-last-month'],
-    run: (income, fromLastMonth) => number(income) + number(fromLastMonth)
+    run: (income, fromLastMonth) =>
+      safeNumber(number(income) + number(fromLastMonth))
   });
 
   sheet.get().createDynamic(sheetName, 'last-month-overspent', {
@@ -104,12 +106,14 @@ export function createSummary(groups, categories, prevSheetName, sheetName) {
     ),
     run: (...data) => {
       data = unflatten2(data);
-      return data.reduce((total, [leftover, carryover]) => {
+      return safeNumber(
-        if (carryover) {
+        data.reduce((total, [leftover, carryover]) => {
-          return total;
+          if (carryover) {
-        }
+            return total;
-        return total + Math.min(0, number(leftover));
+          }
-      }, 0);
+          return total + Math.min(0, number(leftover));
+        }, 0)
+      );
     }
   });
 
@@ -135,11 +139,11 @@ export function createSummary(groups, categories, prevSheetName, sheetName) {
       'buffered'
     ],
     run: (available, lastOverspent, totalBudgeted, buffered) => {
-      return (
+      return safeNumber(
         number(available) +
-        number(lastOverspent) +
+          number(lastOverspent) +
-        number(totalBudgeted) -
+          number(totalBudgeted) -
-        number(buffered)
+          number(buffered)
       );
     }
   });

packages/loot-core/src/server/budget/util.js

@@ -1,11 +1,14 @@
 import { number } from '../spreadsheet/globals';
+import { safeNumber } from '../../shared/util';
 
 export { number } from '../spreadsheet/globals';
 
 export function sumAmounts(...amounts) {
-  return amounts.reduce((total, amount) => {
+  return safeNumber(
-    return total + number(amount);
+    amounts.reduce((total, amount) => {
-  }, 0);
+      return total + number(amount);
+    }, 0)
+  );
 }
 
 export function flatten2(arr) {
@@ -19,23 +22,3 @@ export function unflatten2(arr) {
   }
   return res;
 }
-
-// Note that we don't restrict values to `Number.MIN_SAFE_INTEGER <= value <= Number.MAX_SAFE_INTEGER`
-// where `Number.MAX_SAFE_INTEGER == 2^53 - 1` but a smaller range over `-(2^43-1) <= value <= 2^43 - 1`.
-// This ensure that the number is accurate not just for the integer component but for 3 decimal places also.
-//
-// This gives us the guarantee that can use `safeNumber` on number whether they are unscaled user inputs
-// or they have been converted to integers (using `amountToInteger`).
-
-const MAX_SAFE_NUMBER = 2 ** 43 - 1;
-const MIN_SAFE_NUMBER = -MAX_SAFE_NUMBER;
-
-export function safeNumber(value) {
-  value = number(value);
-  if (value > MAX_SAFE_NUMBER || value < MIN_SAFE_NUMBER) {
-    throw new Error(
-      "Can't safely perform arithmetic with number: " + JSON.stringify(value)
-    );
-  }
-  return value;
-}

packages/loot-core/src/shared/util.js

@@ -298,6 +298,30 @@ export function getNumberFormat() {
 
 setNumberFormat('comma-dot');
 
+// Number utilities
+
+// We dont use `Number.MAX_SAFE_NUMBER` and such here because those
+// numbers are so large that it's not safe to convert them to floats
+// (i.e. N / 100). For example, `9007199254740987 / 100 ===
+// 90071992547409.88`. While the internal arithemetic would be correct
+// because we always do that on numbers, the app would potentially
+// display wrong numbers. Instead of `2**53` we use `2**51` which
+// gives division more room to be correct
+const MAX_SAFE_NUMBER = 2**51 - 1;
+const MIN_SAFE_NUMBER = -MAX_SAFE_NUMBER;
+
+export function safeNumber(value) {
+  if (!Number.isInteger(value)) {
+    throw new Error('safeNumber: number is not an integer: ' + value);
+  }
+  if (value > MAX_SAFE_NUMBER || value < MIN_SAFE_NUMBER) {
+    throw new Error(
+      "safeNumber: can't safely perform arithmetic with number: " + value
+    );
+  }
+  return value;
+}
+
 export function toRelaxedNumber(value) {
   return integerToAmount(currencyToInteger(value) || 0);
 }
@@ -307,8 +331,7 @@ export function toRelaxedInteger(value) {
 }
 
 export function integerToCurrency(n) {
-  // Awesome
+  return numberFormat.formatter.format(safeNumber(n) / 100);
-  return numberFormat.formatter.format(n / 100);
 }
 
 export function amountToCurrency(n) {
@@ -340,7 +363,7 @@ export function amountToInteger(n) {
 }
 
 export function integerToAmount(n) {
-  return parseFloat((n / 100).toFixed(2));
+  return parseFloat((safeNumber(n) / 100).toFixed(2));
 }
 
 // This is used when the input format could be anything (from