Skip to content

Commit

Permalink
Fix: Better min and max range in disburse maturity (#3340)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
lmuntaner authored Sep 20, 2023
1 parent acdf5bd commit 131038d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 4 deletions.
6 changes: 4 additions & 2 deletions frontend/src/lib/modals/neurons/DisburseMaturityModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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",
});
</script>

Expand Down
19 changes: 19 additions & 0 deletions frontend/src/lib/utils/token.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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";
Expand All @@ -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, "'");
Expand Down
26 changes: 24 additions & 2 deletions frontend/src/tests/lib/modals/sns/SnsDisburseMaturityModal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/tests/lib/utils/token.utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 131038d

Please sign in to comment.