From 131038dc9447a6c427aa0bdfe5fe47541718a8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lloren=C3=A7=20Muntaner?= Date: Wed, 20 Sep 2023 15:10:49 +0200 Subject: [PATCH] Fix: Better min and max range in disburse maturity (#3340) # Motivation The range of the predicted tokens in the disbursing flow is an approximation of two decimals. By default, the decimals were rounded the standard way. That meant that the min range could be rounded up and give a wrong min, or the max range rounded down and give a wrong max. In this PR, we add the `roundingMode` parameter to `formatToken` and use it for the range. # Changes * Add `roundingMode` param to `formatToken`. * Use param `roundingMode` for the range in DisburseMaturityModal confirmation screen. # Tests * Add skipped tests because the new parameter is only supported in node 19 and we are in node 18. # Todos - [ ] Add entry to changelog (if necessary). Not necessary. Covered by an entry. --- .../neurons/DisburseMaturityModal.svelte | 6 +++-- frontend/src/lib/utils/token.utils.ts | 19 ++++++++++++++ .../sns/SnsDisburseMaturityModal.spec.ts | 26 +++++++++++++++++-- .../src/tests/lib/utils/token.utils.spec.ts | 11 ++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/modals/neurons/DisburseMaturityModal.svelte b/frontend/src/lib/modals/neurons/DisburseMaturityModal.svelte index 93a8692a4c0..9674e916c3b 100644 --- a/frontend/src/lib/modals/neurons/DisburseMaturityModal.svelte +++ b/frontend/src/lib/modals/neurons/DisburseMaturityModal.svelte @@ -66,11 +66,13 @@ // +/- 5% let predictedMinimumTokens: string; $: predictedMinimumTokens = formatToken({ - value: BigInt(Math.round(Number(maturityToDisburseE8s) * 0.95)), + value: BigInt(Math.floor(Number(maturityToDisburseE8s) * 0.95)), + roundingMode: "floor", }); let predictedMaximumTokens: string; $: predictedMaximumTokens = formatToken({ - value: BigInt(Math.round(Number(maturityToDisburseE8s) * 1.05)), + value: BigInt(Math.ceil(Number(maturityToDisburseE8s) * 1.05)), + roundingMode: "ceil", }); diff --git a/frontend/src/lib/utils/token.utils.ts b/frontend/src/lib/utils/token.utils.ts index e175b2c3a4a..085318be6f4 100644 --- a/frontend/src/lib/utils/token.utils.ts +++ b/frontend/src/lib/utils/token.utils.ts @@ -14,6 +14,18 @@ const countDecimals = (value: number): number => { return Math.max(split[1]?.length ?? 0, ICP_DISPLAYED_DECIMALS); }; +// Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat +type RoundMode = + | "ceil" + | "floor" + | "expand" + | "trunc" + | "halfCeil" + | "halfFloor" + | "halfExpand" + | "halfTrunc" + | "halfEven"; + /** * Jira L2-666: * - If ICP is zero then 0 should be displayed - i.e. without decimals @@ -27,9 +39,11 @@ const countDecimals = (value: number): number => { export const formatToken = ({ value, detailed = false, + roundingMode, }: { value: bigint; detailed?: boolean | "height_decimals"; + roundingMode?: RoundMode; }): string => { if (value === BigInt(0)) { return "0"; @@ -52,6 +66,11 @@ export const formatToken = ({ return new Intl.NumberFormat("en-US", { minimumFractionDigits: decimals, maximumFractionDigits: decimals, + // "roundingMode" not present in `NumberFormatOptions`. + // But it's supported by most modern browsers: https://caniuse.com/mdn-javascript_builtins_intl_numberformat_numberformat_options_roundingmode_parameter + // eslint-disable-next-line + // @ts-ignore + roundingMode, }) .format(converted) .replace(/,/g, "'"); diff --git a/frontend/src/tests/lib/modals/sns/SnsDisburseMaturityModal.spec.ts b/frontend/src/tests/lib/modals/sns/SnsDisburseMaturityModal.spec.ts index e41d9d9459a..79e241f0492 100644 --- a/frontend/src/tests/lib/modals/sns/SnsDisburseMaturityModal.spec.ts +++ b/frontend/src/tests/lib/modals/sns/SnsDisburseMaturityModal.spec.ts @@ -99,15 +99,37 @@ describe("SnsDisburseMaturityModal", () => { }); it("should display summary information in the last step", async () => { - const po = await renderSnsDisburseMaturityModal(); + const neuron = createMockSnsNeuron({ + id: [1], + maturity: 1_000_000_000n, + }); + const po = await renderSnsDisburseMaturityModal(neuron); await po.setPercentage(50); await po.clickNextButton(); expect(await po.getConfirmPercentage()).toBe("50%"); - expect(await po.getConfirmTokens()).toBe("0.48-0.53 TST"); + expect(await po.getConfirmTokens()).toBe("4.75-5.25 TST"); expect(await po.getConfirmDestination()).toBe("Main"); }); + it("should display range with floor and ceil rounding", async () => { + const neuron = createMockSnsNeuron({ + id: [1], + maturity: 123123213n, + }); + const po = await renderSnsDisburseMaturityModal(neuron); + await po.setPercentage(100); + await po.clickNextButton(); + + // NodeJS supports roundingMode since v19 + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#browser_compatibility + // with 123123213n maturity + // -5% is 1,169670524, which should show as 1.16 with the rounding mode "floor" + // +5% is 1,292793737, which should show as 1.30 with the rounding mode "ceil" + // expect(await po.getConfirmTokens()).toBe("1.16-1.30 TST"); + expect(await po.getConfirmTokens()).toBe("1.17-1.29 TST"); + }); + const disburse = async (neuron: SnsNeuron) => { const po = await renderSnsDisburseMaturityModal(neuron); await po.setPercentage(50); diff --git a/frontend/src/tests/lib/utils/token.utils.spec.ts b/frontend/src/tests/lib/utils/token.utils.spec.ts index e6df8de5c69..9eb940643cd 100644 --- a/frontend/src/tests/lib/utils/token.utils.spec.ts +++ b/frontend/src/tests/lib/utils/token.utils.spec.ts @@ -102,6 +102,17 @@ describe("token-utils", () => { ).toEqual(`2'000'000.00000000`); }); + it("should use roundingMode", () => { + // NodeJS supports roundingMode since v19 + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#browser_compatibility + // expect(formatToken({ value: 111_100_000n, roundingMode: "ceil" })).toEqual( + // "1.12" + // ); + expect(formatToken({ value: 111_100_000n, roundingMode: "ceil" })).toEqual( + "1.11" + ); + }); + describe("sumAmountE8s", () => { it("should sum amounts of E8s values", () => { const icp0 = 0n;