Skip to content

Commit

Permalink
Pass ulps to transferTokens (#3936)
Browse files Browse the repository at this point in the history
# Motivation

`transferTokens` currently takes a `amount: number` and optional `token`
to convert the number to `e8s`.
When we use `TokenAmountV2`, the `token` can't be optional because if we
need to use a fallback we can't be certain that we use the token when we
have to.

So instead pass `ulps` to `transferToken` and convert on the calling
side. The calling side knows better whether it's safe to use
`numberToE8s` or whether numbsToUlps` with `token` should be used.

This PR also includes most of the changes from
#3931

# Changes

1. Change the parameters of `transferToken` to accept `amountUlps:
bigint` and don't use `numberToE8s`.
2. Change functions that used to share the params type with
`transferTokens` but which now accept `amount: number` instead of
`amountUlps: bigint` to explicitly state their param types and convert
amount, either with `numberToE8s` for SNS or ckBTC, or with
`numberToUlps` for IcrcToken.
3. Add `numberToUlps`, which requires 8 decimal places until we use
`TokenAmountV2`.

# Tests

1. Copied the unit test from
#3935 but adjust to start
failing when amountToUlps start working instead of skipping that test.
2. Other unit tests are adjust to work with the new param types.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary
  • Loading branch information
dskloetd authored Dec 2, 2023
1 parent b2075c5 commit bcf3198
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import type { NewTransaction } from "$lib/types/transaction";
import TransactionModal from "$lib/modals/transaction/TransactionModal.svelte";
import { replacePlaceholders } from "$lib/utils/i18n.utils";
import { numberToUlps } from "$lib/utils/token.utils";
import type { Account } from "$lib/types/account";
import type { WizardStep } from "@dfinity/gix-components";
import type { TransactionInit } from "$lib/types/transaction";
Expand Down Expand Up @@ -46,8 +47,7 @@
const { blockIndex } = await icrcTransferTokens({
source: sourceAccount,
destinationAddress,
amount,
token,
amountUlps: numberToUlps({ amount, token }),
ledgerCanisterId,
fee: token.fee,
});
Expand Down
14 changes: 10 additions & 4 deletions frontend/src/lib/services/ckbtc-accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import { icrcTransfer } from "$lib/api/icrc-ledger.api";
import { ckBTCTokenStore } from "$lib/derived/universes-tokens.derived";
import { transferTokens } from "$lib/services/icrc-accounts.services";
import { loadAccounts } from "$lib/services/wallet-accounts.services";
import type { Account } from "$lib/types/account";
import type { UniverseCanisterId } from "$lib/types/universe";
import type { Identity } from "@dfinity/agent";
import type { IcrcBlockIndex } from "@dfinity/ledger-icrc";
import { get } from "svelte/store";
import type { IcrcTransferTokensUserParams } from "./icrc-accounts.services";
import { numberToE8s } from "../utils/token.utils";

export const loadCkBTCAccounts = async (params: {
handleError?: () => void;
Expand All @@ -16,19 +17,24 @@ export const loadCkBTCAccounts = async (params: {

export const ckBTCTransferTokens = async ({
source,
destinationAddress,
universeId,
...rest
}: IcrcTransferTokensUserParams & {
amount,
}: {
source: Account;
destinationAddress: string;
universeId: UniverseCanisterId;
amount: number;
}): Promise<{
blockIndex: IcrcBlockIndex | undefined;
}> => {
const fee = get(ckBTCTokenStore)[universeId.toText()]?.token.fee;

return transferTokens({
source,
destinationAddress,
amountUlps: numberToE8s(amount),
fee,
...rest,
transfer: async (
params: {
identity: Identity;
Expand Down
23 changes: 12 additions & 11 deletions frontend/src/lib/services/ckbtc-convert.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ import {
import type { IcrcTransferTokensUserParams } from "./icrc-accounts.services";
import { loadWalletTransactions } from "./wallet-transactions.services";

export type ConvertCkBTCToBtcParams = Omit<
IcrcTransferTokensUserParams,
"source"
> & {
export type ConvertCkBTCToBtcParams = {
destinationAddress: string;
amount: number;
universeId: UniverseCanisterId;
canisters: CkBTCAdditionalCanisters;
updateProgress: (step: ConvertBtcStep) => void;
Expand Down Expand Up @@ -236,13 +235,15 @@ const retrieveBtcAndReload = async ({
canisters: { minterCanisterId, indexCanisterId },
updateProgress,
blockIndex,
}: Omit<IcrcTransferTokensUserParams, "source"> &
Partial<Pick<IcrcTransferTokensUserParams, "source">> & {
universeId: UniverseCanisterId;
canisters: CkBTCAdditionalCanisters;
updateProgress: (step: ConvertBtcStep) => void;
blockIndex?: bigint;
}): Promise<{
}: {
source?: Account;
destinationAddress: string;
amount: number;
universeId: UniverseCanisterId;
canisters: CkBTCAdditionalCanisters;
updateProgress: (step: ConvertBtcStep) => void;
blockIndex?: bigint;
}): Promise<{
success: boolean;
}> => {
updateProgress(ConvertBtcStep.SEND_BTC);
Expand Down
18 changes: 6 additions & 12 deletions frontend/src/lib/services/icrc-accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type { Account } from "$lib/types/account";
import type { IcrcTokenMetadata } from "$lib/types/icrc";
import { notForceCallStrategy } from "$lib/utils/env.utils";
import { ledgerErrorToToastError } from "$lib/utils/sns-ledger.utils";
import { numberToE8s } from "$lib/utils/token.utils";
import type { Identity } from "@dfinity/agent";
import {
decodeIcrcAccount,
Expand All @@ -22,7 +21,7 @@ import {
type IcrcBlockIndex,
} from "@dfinity/ledger-icrc";
import type { Principal } from "@dfinity/principal";
import { isNullish, nonNullish, type Token } from "@dfinity/utils";
import { isNullish, nonNullish } from "@dfinity/utils";
import { get } from "svelte/store";
import { queryAndUpdate } from "./utils.services";

Expand Down Expand Up @@ -145,16 +144,14 @@ export const loadIcrcAccount = ({
export interface IcrcTransferTokensUserParams {
source: Account;
destinationAddress: string;
amount: number;
token?: Token;
amountUlps: bigint;
}

// TODO: use `wallet-accounts.services`
export const transferTokens = async ({
source,
destinationAddress,
amount,
token,
amountUlps,
fee,
transfer,
reloadAccounts,
Expand All @@ -174,15 +171,14 @@ export const transferTokens = async ({
throw new Error("error.transaction_fee_not_found");
}

const amountE8s = numberToE8s(amount, token);
const identity: Identity = await getIcrcAccountIdentity(source);
const to = decodeIcrcAccount(destinationAddress);

const blockIndex = await transfer({
identity,
to,
fromSubAccount: source.subAccount,
amount: amountE8s,
amount: amountUlps,
fee,
});

Expand All @@ -204,8 +200,7 @@ export const transferTokens = async ({
export const icrcTransferTokens = async ({
source,
destinationAddress,
amount,
token,
amountUlps,
fee,
ledgerCanisterId,
}: IcrcTransferTokensUserParams & {
Expand All @@ -214,8 +209,7 @@ export const icrcTransferTokens = async ({
}): Promise<{ blockIndex: IcrcBlockIndex | undefined }> => {
return transferTokens({
source,
amount,
token,
amountUlps,
fee,
destinationAddress,
transfer: async (
Expand Down
13 changes: 9 additions & 4 deletions frontend/src/lib/services/sns-accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import { toastsError } from "$lib/stores/toasts.store";
import { transactionsFeesStore } from "$lib/stores/transaction-fees.store";
import type { Account } from "$lib/types/account";
import { toToastError } from "$lib/utils/error.utils";
import { numberToE8s } from "$lib/utils/token.utils";
import type { Identity } from "@dfinity/agent";
import type { IcrcBlockIndex } from "@dfinity/ledger-icrc";
import type { Principal } from "@dfinity/principal";
import { get } from "svelte/store";
import type { IcrcTransferTokensUserParams } from "./icrc-accounts.services";
import { loadSnsAccountTransactions } from "./sns-transactions.services";
import { loadSnsTransactionFee } from "./transaction-fees.services";
import { queryAndUpdate } from "./utils.services";
Expand Down Expand Up @@ -73,18 +73,23 @@ export const syncSnsAccounts = async (params: {
export const snsTransferTokens = async ({
rootCanisterId,
source,
destinationAddress,
amount,
loadTransactions,
...rest
}: IcrcTransferTokensUserParams & {
}: {
rootCanisterId: Principal;
source: Account;
destinationAddress: string;
amount: number;
loadTransactions: boolean;
}): Promise<{ blockIndex: IcrcBlockIndex | undefined }> => {
const fee = get(transactionsFeesStore).projects[rootCanisterId.toText()]?.fee;

return transferTokens({
source,
destinationAddress,
amountUlps: numberToE8s(amount),
fee,
...rest,
transfer: async (
params: {
identity: Identity;
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/lib/utils/token.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,23 @@ export const numberToE8s = (amount: number, token: Token = ICPToken): bigint =>
token,
}).toE8s();

/**
* Returns the number of Ulps for the given amount.
*
* The precision is given by the token.
*
* @param {Object} params
* @param {number} parms.amount
* @param {token} params.token
* @returns {bigint}
* @throws {Error} If the amount has more than number of decimals in the token.
*/
// TODO: Use TokenAmountV2
export const numberToUlps = (params: {
amount: number;
token: Token;
}): bigint => TokenAmount.fromNumber(params).toE8s();

export class UnavailableTokenAmount {
public token: Token;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ describe("icrc-accounts-services", () => {
});

describe("icrcTransferTokens", () => {
const amount = 10;
const amountE8s = BigInt(10 * E8S_PER_ICP);
const fee = 10_000n;
const destinationAccount = {
Expand All @@ -214,7 +213,7 @@ describe("icrc-accounts-services", () => {
it("calls icrcTransfer from icrc ledger api", async () => {
await icrcTransferTokens({
source: mockIcrcMainAccount,
amount,
amountUlps: amountE8s,
destinationAddress: encodeIcrcAccount(destinationAccount),
fee,
ledgerCanisterId,
Expand All @@ -237,7 +236,7 @@ describe("icrc-accounts-services", () => {
type: "subAccount",
subAccount: mockSubAccountArray,
},
amount,
amountUlps: amountE8s,
destinationAddress: encodeIcrcAccount(destinationAccount),
fee,
ledgerCanisterId,
Expand Down Expand Up @@ -269,7 +268,7 @@ describe("icrc-accounts-services", () => {

await icrcTransferTokens({
source: mockIcrcMainAccount,
amount,
amountUlps: amountE8s,
destinationAddress: encodeIcrcAccount(destinationAccount),
fee,
ledgerCanisterId,
Expand Down
46 changes: 35 additions & 11 deletions frontend/src/tests/lib/utils/token.utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
formatToken,
getMaxTransactionAmount,
numberToE8s,
numberToUlps,
sumAmountE8s,
} from "$lib/utils/token.utils";
import { mockCkETHToken } from "$tests/mocks/cketh-accounts.mock";
import { ICPToken, TokenAmount } from "@dfinity/utils";

describe("token-utils", () => {
Expand Down Expand Up @@ -261,17 +261,41 @@ describe("token-utils", () => {
expect(numberToE8s(3.14)).toBe(BigInt(314_000_000));
expect(numberToE8s(0.14)).toBe(BigInt(14_000_000));
});
});

// TODO: Enable when we upgrade ic-js with TokenAmount supporting decimals.
it.skip("converts number to e8s with token", () => {
expect(numberToE8s(1.14, mockCkETHToken)).toBe(
BigInt(1_140_000_000_000_000_000)
);
expect(numberToE8s(1, mockCkETHToken)).toBe(
BigInt(1_000_000_000_000_000_000)
);
expect(numberToE8s(3.14, mockCkETHToken)).toBe(
BigInt(3_140_000_000_000_000_000)
describe("numberToUlps", () => {
it("converts number to e8s", () => {
const token = {
decimals: 8,
symbol: "TEST",
name: "Test",
};
expect(numberToUlps({ amount: 1.14, token })).toBe(114_000_000n);
expect(numberToUlps({ amount: 1, token })).toBe(100_000_000n);
expect(numberToUlps({ amount: 3.14, token })).toBe(314_000_000n);
expect(numberToUlps({ amount: 0.14, token })).toBe(14_000_000n);
});

it("converts number to ulps with token", () => {
const token = {
decimals: 18,
symbol: "TEST",
name: "Test",
};
// TODO: Move these outside the call once they pass.
const call = () => {
expect(numberToUlps({ amount: 1.14, token })).toBe(
1_140_000_000_000_000_000n
);
expect(numberToUlps({ amount: 1, token })).toBe(
1_000_000_000_000_000_000n
);
expect(numberToUlps({ amount: 3.14, token })).toBe(
3_140_000_000_000_000_000n
);
};
expect(call).toThrowError(
"Use TokenAmountV2 for number of decimals other than 8"
);
});
});
Expand Down

0 comments on commit bcf3198

Please sign in to comment.