From 3a4e4f0301100b613f50439bf1cce01eacc86fdb Mon Sep 17 00:00:00 2001 From: David Dal Busco Date: Fri, 8 Nov 2024 08:52:55 +0100 Subject: [PATCH] feat(frontend): optional index canister (#3334) # Motivation Make the index canister ID optional in the core type definition without changing the way we load information - i.e. in a backwards compatible manner. # Notes This PR is relatively large, so Antonio and I reviewed the code together offline. # Changes - `custom-token.services.ts`: extend `toCustomToken` to support optional Index canister ID - `IcTransactions.svelte`: silently ignore listing transactions if no index - `ic-token.schema.ts`: set Index Canister as optional in the base type `IcCanisters` - `ic-add-custom-tokens.service.ts`: extend support to, in the future, to allow user to register custom token without index canister id - `icrc-wallet.worker.ts`: assert index is set. in the future this should not be called without index - add few TODOs - adapt tests --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- .../icp-eth/services/custom-token.services.ts | 6 +- .../transactions/IcTransactions.svelte | 7 + src/frontend/src/icp/derived/icrc.derived.ts | 2 +- .../src/icp/schema/ic-token.schema.ts | 3 +- .../services/ic-add-custom-tokens.service.ts | 62 ++-- .../icp/services/ic-transactions.services.ts | 6 +- .../src/icp/services/icrc.services.ts | 1 + .../src/icp/workers/icrc-wallet.worker.ts | 7 +- src/frontend/src/lib/i18n/en.json | 1 + src/frontend/src/lib/types/i18n.d.ts | 1 + .../services/custom-token.services.spec.ts | 323 +++++++++-------- .../tests/icp/schema/ic-token.schema.spec.ts | 22 +- .../ic-add-custom-tokens.service.spec.ts | 327 ++++++++++++------ .../validation/ic-token.validation.spec.ts | 25 +- .../src/tests/mocks/ic-tokens.mock.ts | 6 +- 15 files changed, 486 insertions(+), 313 deletions(-) diff --git a/src/frontend/src/icp-eth/services/custom-token.services.ts b/src/frontend/src/icp-eth/services/custom-token.services.ts index eaa2b5b33f..36894be300 100644 --- a/src/frontend/src/icp-eth/services/custom-token.services.ts +++ b/src/frontend/src/icp-eth/services/custom-token.services.ts @@ -10,7 +10,7 @@ import type { OptionIdentity } from '$lib/types/identity'; import type { Token } from '$lib/types/token'; import type { Identity } from '@dfinity/agent'; import { Principal } from '@dfinity/principal'; -import { isNullish, toNullable } from '@dfinity/utils'; +import { isNullish, nonNullish, toNullable } from '@dfinity/utils'; import { get } from 'svelte/store'; const assertErc20SendTokenData = (sendToken: Erc20Token): AutoLoadTokenResult | undefined => { @@ -72,7 +72,9 @@ export const toCustomToken = ({ token: { Icrc: { ledger_id: Principal.fromText(ledgerCanisterId), - index_id: toNullable(Principal.fromText(indexCanisterId)) + index_id: toNullable( + nonNullish(indexCanisterId) ? Principal.fromText(indexCanisterId) : undefined + ) } } }); diff --git a/src/frontend/src/icp/components/transactions/IcTransactions.svelte b/src/frontend/src/icp/components/transactions/IcTransactions.svelte index e8a68bc21c..62cb8a038e 100644 --- a/src/frontend/src/icp/components/transactions/IcTransactions.svelte +++ b/src/frontend/src/icp/components/transactions/IcTransactions.svelte @@ -22,6 +22,7 @@ import { icTransactions } from '$icp/derived/ic-transactions.derived'; import { loadNextTransactions } from '$icp/services/ic-transactions.services'; import type { IcTransactionUi } from '$icp/types/ic-transaction'; + import { isNotIcToken, isNotIcTokenCanistersStrict } from '$icp/validation/ic-token.validation'; import TransactionsPlaceholder from '$lib/components/transactions/TransactionsPlaceholder.svelte'; import Header from '$lib/components/ui/Header.svelte'; import { WALLET_PAGINATION } from '$lib/constants/app.constants'; @@ -69,6 +70,12 @@ return; } + if (isNotIcToken($tokenAsIcToken) || isNotIcTokenCanistersStrict($tokenAsIcToken)) { + // On one hand, we assume that the parent component does not mount this component if no transactions can be fetched; on the other hand, we want to avoid displaying an error toast that could potentially appear multiple times. + // Therefore, we do not particularly display a visual error. In any case, we cannot load transactions without an Index canister. + return; + } + await loadNextTransactions({ owner: $authIdentity.getPrincipal(), identity: $authIdentity, diff --git a/src/frontend/src/icp/derived/icrc.derived.ts b/src/frontend/src/icp/derived/icrc.derived.ts index a12d99ee97..aba3edd297 100644 --- a/src/frontend/src/icp/derived/icrc.derived.ts +++ b/src/frontend/src/icp/derived/icrc.derived.ts @@ -71,7 +71,7 @@ const icrcDefaultTokensToggleable: Readable = derived( userLedgerCanisterId === ledgerCanisterId && userIndexCanisterId === indexCanisterId ); - return mapDefaultTokenToToggleable({ + return mapDefaultTokenToToggleable({ defaultToken: { ledgerCanisterId, indexCanisterId, diff --git a/src/frontend/src/icp/schema/ic-token.schema.ts b/src/frontend/src/icp/schema/ic-token.schema.ts index 9630ee0dd5..58bcf682e2 100644 --- a/src/frontend/src/icp/schema/ic-token.schema.ts +++ b/src/frontend/src/icp/schema/ic-token.schema.ts @@ -16,8 +16,7 @@ export const IcAppMetadataSchema = z.object({ export const IcCanistersSchema = z.object({ ledgerCanisterId: CanisterIdTextSchema, - // TODO: Make canister .optional() - indexCanisterId: CanisterIdTextSchema + indexCanisterId: CanisterIdTextSchema.optional() }); export const IcCanistersStrictSchema = IcCanistersSchema.extend({ diff --git a/src/frontend/src/icp/services/ic-add-custom-tokens.service.ts b/src/frontend/src/icp/services/ic-add-custom-tokens.service.ts index 7b93c17e58..c51b37324f 100644 --- a/src/frontend/src/icp/services/ic-add-custom-tokens.service.ts +++ b/src/frontend/src/icp/services/ic-add-custom-tokens.service.ts @@ -1,12 +1,12 @@ import { getLedgerId, getTransactions as getTransactionsIcrc } from '$icp/api/icrc-index-ng.api'; -import { metadata } from '$icp/api/icrc-ledger.api'; +import { balance, metadata } from '$icp/api/icrc-ledger.api'; import type { IcCanisters, IcToken, IcTokenWithoutId } from '$icp/types/ic-token'; import { mapIcrcToken } from '$icp/utils/icrc.utils'; import { i18n } from '$lib/stores/i18n.store'; import { toastsError } from '$lib/stores/toasts.store'; import type { OptionIdentity } from '$lib/types/identity'; import type { Identity } from '@dfinity/agent'; -import { assertNonNullish, isNullish } from '@dfinity/utils'; +import { assertNonNullish, isNullish, nonNullish } from '@dfinity/utils'; import { get } from 'svelte/store'; export interface ValidateTokenData { @@ -35,13 +35,6 @@ export const loadAndAssertAddCustomToken = async ({ return { result: 'error' }; } - if (isNullish(indexCanisterId)) { - toastsError({ - msg: { text: get(i18n).tokens.import.error.missing_index_id } - }); - return { result: 'error' }; - } - const canisterIds = { ledgerCanisterId, indexCanisterId }; const { alreadyAvailable } = assertAlreadyAvailable({ @@ -53,22 +46,26 @@ export const loadAndAssertAddCustomToken = async ({ return { result: 'error' }; } - const { valid } = await assertLedgerId({ - identity, - ...canisterIds - }); + const { valid } = nonNullish(indexCanisterId) + ? await assertIndexLedgerId({ + identity, + ...canisterIds, + indexCanisterId + }) + : { valid: true }; if (!valid) { return { result: 'error' }; } try { + const params = { identity, ...canisterIds }; + const [token, balance] = await Promise.all([ - loadMetadata({ - identity, - ...canisterIds - }), - loadBalance({ identity, ...canisterIds }) + loadMetadata(params), + ...(isNullish(indexCanisterId) + ? [loadLedgerBalance(params)] + : [loadIndexBalance({ ...params, indexCanisterId })]) ]); if (isNullish(token)) { @@ -157,10 +154,31 @@ const loadMetadata = async ({ } }; -const loadBalance = async ({ +const loadLedgerBalance = async ({ + identity, + ledgerCanisterId +}: IcCanisters & { identity: Identity }): Promise => { + try { + return await balance({ + ledgerCanisterId, + identity, + owner: identity.getPrincipal(), + certified: true + }); + } catch (err: unknown) { + toastsError({ + msg: { text: get(i18n).tokens.import.error.unexpected_ledger }, + err + }); + + throw err; + } +}; + +const loadIndexBalance = async ({ identity, indexCanisterId -}: Pick & { identity: Identity }): Promise => { +}: Required> & { identity: Identity }): Promise => { try { const { balance } = await getTransactionsIcrc({ indexCanisterId, @@ -181,11 +199,11 @@ const loadBalance = async ({ } }; -const assertLedgerId = async ({ +const assertIndexLedgerId = async ({ identity, indexCanisterId, ledgerCanisterId -}: IcCanisters & { identity: Identity }): Promise<{ valid: boolean }> => { +}: Required & { identity: Identity }): Promise<{ valid: boolean }> => { try { const ledgerId = await getLedgerId({ indexCanisterId, diff --git a/src/frontend/src/icp/services/ic-transactions.services.ts b/src/frontend/src/icp/services/ic-transactions.services.ts index 09bd1c945f..88ded76762 100644 --- a/src/frontend/src/icp/services/ic-transactions.services.ts +++ b/src/frontend/src/icp/services/ic-transactions.services.ts @@ -1,7 +1,7 @@ import { getTransactions as getTransactionsIcp } from '$icp/api/icp-index.api'; import { getTransactions as getTransactionsIcrc } from '$icp/api/icrc-index-ng.api'; import { icTransactionsStore } from '$icp/stores/ic-transactions.store'; -import type { IcToken } from '$icp/types/ic-token'; +import type { IcCanistersStrict, IcToken } from '$icp/types/ic-token'; import type { IcTransaction } from '$icp/types/ic-transaction'; import { mapIcTransaction } from '$icp/utils/ic-transactions.utils'; import { mapTransactionIcpToSelf } from '$icp/utils/icp-transactions.utils'; @@ -23,7 +23,7 @@ const getTransactions = async ({ identity: OptionIdentity; start?: bigint; maxResults?: bigint; - token: IcToken; + token: IcToken & IcCanistersStrict; }): Promise => { if (standard === 'icrc') { const { transactions } = await getTransactionsIcrc({ @@ -49,7 +49,7 @@ export const loadNextTransactions = ({ identity: OptionIdentity; start?: bigint; maxResults?: bigint; - token: IcToken; + token: IcToken & IcCanistersStrict; signalEnd: () => void; }): Promise => queryAndUpdate({ diff --git a/src/frontend/src/icp/services/icrc.services.ts b/src/frontend/src/icp/services/icrc.services.ts index 2807bbc6ad..95a04692b1 100644 --- a/src/frontend/src/icp/services/icrc.services.ts +++ b/src/frontend/src/icp/services/icrc.services.ts @@ -152,6 +152,7 @@ const loadCustomIcrcTokensData = async ({ const indexCanisterId = fromNullable(index_id); + // TODO(OISY-296): remove isNullish(indexCanisterId) when support for reading balance and no index is fully implemented // Index canister ID currently mandatory in Oisy's frontend if (isNullish(indexCanisterId)) { return undefined; diff --git a/src/frontend/src/icp/workers/icrc-wallet.worker.ts b/src/frontend/src/icp/workers/icrc-wallet.worker.ts index 02807464ea..1d10039eb9 100644 --- a/src/frontend/src/icp/workers/icrc-wallet.worker.ts +++ b/src/frontend/src/icp/workers/icrc-wallet.worker.ts @@ -25,13 +25,18 @@ const getTransactions = ({ }: SchedulerJobParams): Promise => { assertNonNullish(data, 'No data - indexCanisterId - provided to fetch transactions.'); + // TODO(OISY-296): This is not clean. If the index canister ID is not provided we should not even land here. + const { indexCanisterId } = data; + assertNonNullish(indexCanisterId); + return getTransactionsApi({ identity, certified, owner: identity.getPrincipal(), // We query tip to discover the new transactions start: undefined, - ...data + ...data, + indexCanisterId }); }; diff --git a/src/frontend/src/lib/i18n/en.json b/src/frontend/src/lib/i18n/en.json index 46d34ab7a3..125fb51212 100644 --- a/src/frontend/src/lib/i18n/en.json +++ b/src/frontend/src/lib/i18n/en.json @@ -435,6 +435,7 @@ "error": { "loading_metadata": "Error while loading the metadata of the token.", "no_metadata": "No metadata for the token is provided. This is unexpected.", + "unexpected_ledger": "Something went wrong while validating the Ledger canister.", "unexpected_index": "Something went wrong while validating the Index canister.", "unexpected_index_ledger": "Something went wrong while loading the Ledger ID related to the Index canister.", "invalid_ledger_id": "The Ledger ID is not related to the Index canister.", diff --git a/src/frontend/src/lib/types/i18n.d.ts b/src/frontend/src/lib/types/i18n.d.ts index 346efd2c99..6b6067bb35 100644 --- a/src/frontend/src/lib/types/i18n.d.ts +++ b/src/frontend/src/lib/types/i18n.d.ts @@ -396,6 +396,7 @@ interface I18nTokens { error: { loading_metadata: string; no_metadata: string; + unexpected_ledger: string; unexpected_index: string; unexpected_index_ledger: string; invalid_ledger_id: string; diff --git a/src/frontend/src/tests/icp-eth/services/custom-token.services.spec.ts b/src/frontend/src/tests/icp-eth/services/custom-token.services.spec.ts index c3e505a47c..49f3c6f1ea 100644 --- a/src/frontend/src/tests/icp-eth/services/custom-token.services.spec.ts +++ b/src/frontend/src/tests/icp-eth/services/custom-token.services.spec.ts @@ -18,7 +18,7 @@ import { mockValidToken } from '$tests/mocks/tokens.mock'; import type { HttpAgent } from '@dfinity/agent'; import { IcrcLedgerCanister } from '@dfinity/ledger-icrc'; import { Principal } from '@dfinity/principal'; -import { toNullable } from '@dfinity/utils'; +import { isNullish, toNullable } from '@dfinity/utils'; import { get } from 'svelte/store'; import { expect, type MockInstance } from 'vitest'; import { mock } from 'vitest-mock-extended'; @@ -65,10 +65,12 @@ describe('custom-token.services', () => { describe('success', () => { const assertSetCustomToken = async ({ customTokens, - expectedVersion + expectedVersion, + indexCanisterId }: { customTokens: IcrcCustomToken[]; expectedVersion: [] | [bigint]; + indexCanisterId: string | undefined; }) => { const spySetCustomToken = backendCanisterMock.setCustomToken.mockResolvedValue(undefined); const spyListCustomTokens = backendCanisterMock.listCustomTokens.mockResolvedValue([]); @@ -79,8 +81,11 @@ describe('custom-token.services', () => { standard: 'erc20' as const }; + const [first, ...rest] = customTokens; + const icrcCustomTokens = [{ ...first, indexCanisterId }, ...rest]; + const { result } = await autoLoadCustomToken({ - icrcCustomTokens: customTokens, + icrcCustomTokens, sendToken: mockSendToken, identity: mockIdentity }); @@ -93,7 +98,7 @@ describe('custom-token.services', () => { version: expectedVersion, token: { Icrc: { - index_id: [Principal.fromText(mockSendToken.indexCanisterId)], + index_id: isNullish(indexCanisterId) ? [] : [Principal.fromText(indexCanisterId)], ledger_id: Principal.fromText(mockSendToken.ledgerCanisterId) } } @@ -103,77 +108,92 @@ describe('custom-token.services', () => { expect(spyListCustomTokens).toHaveBeenCalledWith({ certified: true }); }; - it('should call setCustomToken with a new custom token', async () => { - await assertSetCustomToken({ customTokens: mockIcrcCustomTokens, expectedVersion: [] }); - }); - - it('should call setCustomToken to update a custom token', async () => { - const customTokens: IcrcCustomToken[] = [ - { - ...mockIcrcCustomTokens[0], - version: 1n - }, - mockIcrcCustomTokens[1] - ]; - - await assertSetCustomToken({ - customTokens, - expectedVersion: [customTokens[0].version ?? 0n] - }); - }); - - it('should load tokens after set custom token', async () => { - backendCanisterMock.setCustomToken.mockResolvedValue(undefined); - const spyListCustomTokens = backendCanisterMock.listCustomTokens.mockResolvedValue([ - { - token: { - Icrc: { - index_id: [Principal.fromText(mockValidSendToken.indexCanisterId)], - ledger_id: Principal.fromText(mockValidSendToken.ledgerCanisterId) - } - }, - version: [1n], - enabled: true - } - ]); - - const spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ - ['icrc1:name', { Text: mockValidSendToken.name }], - ['icrc1:symbol', { Text: mockValidSendToken.symbol }], - ['icrc1:decimals', { Nat: BigInt(mockValidSendToken.decimals) }], - ['icrc1:fee', { Nat: mockValidSendToken.fee }] - ]); - - const { result } = await autoLoadCustomToken({ - icrcCustomTokens: mockIcrcCustomTokens, - sendToken: mockValidSendToken, - identity: mockIdentity - }); - - expect(result).toBe('loaded'); - - expect(spyListCustomTokens).toHaveBeenCalledWith({ certified: true }); - - expect(spyMetadata).toHaveBeenCalledWith({ certified: true }); - - const store = get(icrcCustomTokensStore); - - expect(store).toHaveLength(1); - expect(store).toEqual([ - { - certified: true, - data: expect.objectContaining({ - ...mockValidIcToken, - id: expect.any(Symbol), - category: 'custom', - position: 4, - enabled: true, - standard: 'icrc', + it.each([undefined, IC_CKBTC_INDEX_CANISTER_ID])( + 'should call setCustomToken with a new custom token with index %s', + async (indexCanisterId) => { + await assertSetCustomToken({ + customTokens: mockIcrcCustomTokens, + expectedVersion: [], + indexCanisterId + }); + } + ); + + it.each([undefined, IC_CKBTC_INDEX_CANISTER_ID])( + 'should call setCustomToken to update a custom token with index %s', + async (indexCanisterId) => { + const customTokens: IcrcCustomToken[] = [ + { + ...mockIcrcCustomTokens[0], version: 1n - }) - } - ]); - }); + }, + mockIcrcCustomTokens[1] + ]; + + await assertSetCustomToken({ + customTokens, + expectedVersion: [customTokens[0].version ?? 0n], + indexCanisterId + }); + } + ); + + // TODO(OISY-296): introduce test for undefined Index canister ID + it.each([IC_CKBTC_INDEX_CANISTER_ID])( + 'should load tokens after set custom token with index ID %s', + async (indexCanisterId) => { + backendCanisterMock.setCustomToken.mockResolvedValue(undefined); + const spyListCustomTokens = backendCanisterMock.listCustomTokens.mockResolvedValue([ + { + token: { + Icrc: { + index_id: isNullish(indexCanisterId) ? [] : [Principal.fromText(indexCanisterId)], + ledger_id: Principal.fromText(mockValidSendToken.ledgerCanisterId) + } + }, + version: [1n], + enabled: true + } + ]); + + const spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ + ['icrc1:name', { Text: mockValidSendToken.name }], + ['icrc1:symbol', { Text: mockValidSendToken.symbol }], + ['icrc1:decimals', { Nat: BigInt(mockValidSendToken.decimals) }], + ['icrc1:fee', { Nat: mockValidSendToken.fee }] + ]); + + const { result } = await autoLoadCustomToken({ + icrcCustomTokens: mockIcrcCustomTokens, + sendToken: mockValidSendToken, + identity: mockIdentity + }); + + expect(result).toBe('loaded'); + + expect(spyListCustomTokens).toHaveBeenCalledWith({ certified: true }); + + expect(spyMetadata).toHaveBeenCalledWith({ certified: true }); + + const store = get(icrcCustomTokensStore); + + expect(store).toHaveLength(1); + expect(store).toEqual([ + { + certified: true, + data: expect.objectContaining({ + ...mockValidIcToken, + id: expect.any(Symbol), + category: 'custom', + position: 4, + enabled: true, + standard: 'icrc', + version: 1n + }) + } + ]); + } + ); }); describe('error', () => { @@ -217,89 +237,110 @@ describe('custom-token.services', () => { }); }); - it('should result with loaded but toastError if metadata fails', async () => { - backendCanisterMock.setCustomToken.mockResolvedValue(undefined); - - backendCanisterMock.listCustomTokens.mockResolvedValue([ - { - token: { - Icrc: { - index_id: [Principal.fromText(mockValidSendToken.indexCanisterId)], - ledger_id: Principal.fromText(mockValidSendToken.ledgerCanisterId) - } - }, - version: [1n], - enabled: true - } - ]); + // TODO(OISY-296): introduce test for undefined Index canister ID + it.each([IC_CKBTC_INDEX_CANISTER_ID])( + 'should result with loaded but toastError if metadata fails with index ID %s', + async (indexCanisterId) => { + backendCanisterMock.setCustomToken.mockResolvedValue(undefined); + + backendCanisterMock.listCustomTokens.mockResolvedValue([ + { + token: { + Icrc: { + index_id: isNullish(indexCanisterId) ? [] : [Principal.fromText(indexCanisterId)], + ledger_id: Principal.fromText(mockValidSendToken.ledgerCanisterId) + } + }, + version: [1n], + enabled: true + } + ]); - const err = new Error('test'); - ledgerCanisterMock.metadata.mockRejectedValue(err); + const err = new Error('test'); + ledgerCanisterMock.metadata.mockRejectedValue(err); - const { result } = await autoLoadCustomToken({ - icrcCustomTokens: mockIcrcCustomTokens, - sendToken: mockValidSendToken, - identity: mockIdentity - }); + const { result } = await autoLoadCustomToken({ + icrcCustomTokens: mockIcrcCustomTokens, + sendToken: mockValidSendToken, + identity: mockIdentity + }); - expect(result).toBe('loaded'); + expect(result).toBe('loaded'); - expect(spyToastsError).toHaveBeenNthCalledWith(1, { - msg: { text: get(i18n).init.error.icrc_canisters }, - err - }); - }); + expect(spyToastsError).toHaveBeenNthCalledWith(1, { + msg: { text: get(i18n).init.error.icrc_canisters }, + err + }); + } + ); }); }); describe('toCustomToken', () => { - it.each([undefined, 2n])('should convert to CustomToken with version %s', (version) => { - const input: SaveCustomToken = { - enabled: true, - version, - ledgerCanisterId: IC_CKBTC_LEDGER_CANISTER_ID, - indexCanisterId: IC_CKBTC_INDEX_CANISTER_ID - }; + describe.each([undefined, IC_CKBTC_INDEX_CANISTER_ID])( + 'with index ID %s', + (indexCanisterId) => { + it.each([undefined, 2n])('should convert to CustomToken with version %s', (version) => { + const input: SaveCustomToken = { + enabled: true, + version, + ledgerCanisterId: IC_CKBTC_LEDGER_CANISTER_ID, + indexCanisterId + }; - const result = toCustomToken(input); + const result = toCustomToken(input); - expect(result).toEqual({ - enabled: input.enabled, - version: toNullable(version), - token: { - Icrc: { - ledger_id: Principal.fromText(input.ledgerCanisterId), - index_id: [Principal.fromText(input.indexCanisterId)] - } - } - }); - }); + expect(result).toEqual({ + enabled: input.enabled, + version: toNullable(version), + token: { + Icrc: { + ledger_id: Principal.fromText(input.ledgerCanisterId), + index_id: toNullable( + isNullish(indexCanisterId) ? undefined : Principal.fromText(indexCanisterId) + ) + } + } + }); + }); + } + ); }); describe('setCustomToken', () => { - it('should call backend setCustomToken with expected parameters', async () => { - const spySetCustomToken = backendCanisterMock.setCustomToken.mockResolvedValue(undefined); + describe.each([undefined, IC_CKBTC_INDEX_CANISTER_ID])( + 'with index ID %s', + (indexCanisterId) => { + it('should call backend setCustomToken with expected parameters', async () => { + const spySetCustomToken = backendCanisterMock.setCustomToken.mockResolvedValue(undefined); - const enabled = true; + const enabled = true; - await setCustomToken({ - token: mockIcrcCustomToken, - identity: mockIdentity, - enabled - }); + await setCustomToken({ + token: { + ...mockIcrcCustomToken, + indexCanisterId + }, + identity: mockIdentity, + enabled + }); - expect(spySetCustomToken).toHaveBeenCalledWith({ - token: { - enabled, - version: toNullable(mockIcrcCustomToken.version), - token: { - Icrc: { - ledger_id: Principal.fromText(mockIcrcCustomToken.ledgerCanisterId), - index_id: [Principal.fromText(mockIcrcCustomToken.indexCanisterId)] + expect(spySetCustomToken).toHaveBeenCalledWith({ + token: { + enabled, + version: toNullable(mockIcrcCustomToken.version), + token: { + Icrc: { + ledger_id: Principal.fromText(mockIcrcCustomToken.ledgerCanisterId), + index_id: toNullable( + isNullish(indexCanisterId) ? undefined : Principal.fromText(indexCanisterId) + ) + } + } } - } - } - }); - }); + }); + }); + } + ); }); }); diff --git a/src/frontend/src/tests/icp/schema/ic-token.schema.spec.ts b/src/frontend/src/tests/icp/schema/ic-token.schema.spec.ts index 992204bf2a..a426ef89b1 100644 --- a/src/frontend/src/tests/icp/schema/ic-token.schema.spec.ts +++ b/src/frontend/src/tests/icp/schema/ic-token.schema.spec.ts @@ -99,13 +99,12 @@ describe('ic-token.schema', () => { expect(IcCanistersSchema.parse(validData)).toEqual(validData); }); - // TODO: uncomment when Index canister becomes optional - // it('should validate with ledger canister only', () => { - // const validData = { - // ledgerCanisterId: mockCanisters.ledgerCanisterId - // }; - // expect(IcCanistersSchema.parse(validData)).toEqual(validData); - // }); + it('should validate with ledger canister only', () => { + const validData = { + ledgerCanisterId: mockCanisters.ledgerCanisterId + }; + expect(IcCanistersSchema.parse(validData)).toEqual(validData); + }); it('should fail with invalid ledger canister id', () => { const invalidData = { @@ -219,11 +218,10 @@ describe('ic-token.schema', () => { expect(IcInterfaceSchema.parse(validData)).toEqual(validData); }); - // TODO: uncomment when Index canister becomes optional - // it('should validate without Index canister', () => { - // const { indexCanisterId: _, ...restValidData } = validData; - // expect(IcInterfaceSchema.parse(restValidData)).toEqual(restValidData); - // }); + it('should validate without Index canister', () => { + const { indexCanisterId: _, ...restValidData } = validData; + expect(IcInterfaceSchema.parse(restValidData)).toEqual(restValidData); + }); it('should fail with incorrect IcCanisters data', () => { const invalidData = { diff --git a/src/frontend/src/tests/icp/services/ic-add-custom-tokens.service.spec.ts b/src/frontend/src/tests/icp/services/ic-add-custom-tokens.service.spec.ts index b34bfb948f..b15196a45d 100644 --- a/src/frontend/src/tests/icp/services/ic-add-custom-tokens.service.spec.ts +++ b/src/frontend/src/tests/icp/services/ic-add-custom-tokens.service.spec.ts @@ -1,12 +1,13 @@ import { ICP_NETWORK } from '$env/networks.env'; import { loadAndAssertAddCustomToken } from '$icp/services/ic-add-custom-tokens.service'; -import type { IcToken } from '$icp/types/ic-token'; +import type { IcCanisters, IcToken } from '$icp/types/ic-token'; import { getIcrcAccount } from '$icp/utils/icrc-account.utils'; import * as agent from '$lib/actors/agents.ic'; import { i18n } from '$lib/stores/i18n.store'; import * as toastsStore from '$lib/stores/toasts.store'; +import type { OptionIdentity } from '$lib/types/identity'; import { parseTokenId } from '$lib/validation/token.validation'; -import { mockIdentity } from '$tests/mocks/identity.mock'; +import { mockIdentity, mockPrincipal } from '$tests/mocks/identity.mock'; import type { HttpAgent } from '@dfinity/agent'; import { IcrcIndexNgCanister, IcrcLedgerCanister } from '@dfinity/ledger-icrc'; import { Principal } from '@dfinity/principal'; @@ -30,12 +31,12 @@ describe('ic-add-custom-tokens.service', () => { let spyLedgerId: MockInstance; let spyGetTransactions: MockInstance; let spyMetadata: MockInstance; + let spyBalance: MockInstance; const validParams = { identity: mockIdentity, icrcTokens: [], - ledgerCanisterId: mockLedgerCanisterId, - indexCanisterId: mockIndexCanisterId + ledgerCanisterId: mockLedgerCanisterId }; const tokenName = 'Test Token'; @@ -97,20 +98,6 @@ describe('ic-add-custom-tokens.service', () => { }); }); - it('should return error if indexCanisterId is missing', async () => { - const result = await loadAndAssertAddCustomToken({ - identity: mockIdentity, - icrcTokens: [], - ledgerCanisterId: mockLedgerCanisterId - }); - - expect(result).toEqual({ result: 'error' }); - - expect(spyToastsError).toHaveBeenNthCalledWith(1, { - msg: { text: get(i18n).tokens.import.error.missing_index_id } - }); - }); - it('should return error if token is already available', async () => { const result = await loadAndAssertAddCustomToken({ identity: mockIdentity, @@ -140,81 +127,118 @@ describe('ic-add-custom-tokens.service', () => { }); }); - it('should return error if ledger is not related to index', async () => { - indexCanisterMock.ledgerId.mockResolvedValue( - Principal.fromText('2ouva-viaaa-aaaaq-aaamq-cai') - ); + describe('without index canister', () => { + it('should return error if metadata are undefined', async () => { + spyBalance = ledgerCanisterMock.balance.mockResolvedValue(123n); - const result = await loadAndAssertAddCustomToken(validParams); + spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([]); - expect(result).toEqual({ result: 'error' }); - }); + const result = await loadAndAssertAddCustomToken(validParams); - it('should return error if metadata are undefined', async () => { - spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( - Principal.fromText(mockLedgerCanisterId) - ); + expect(result).toEqual({ result: 'error' }); - spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ - balance: 100n, - transactions: [], - oldest_tx_id: [0n] + expect(spyToastsError).toHaveBeenNthCalledWith(1, { + msg: { text: get(i18n).tokens.import.error.no_metadata } + }); }); - spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([]); + it('should return error if token already exits', async () => { + spyBalance = ledgerCanisterMock.balance.mockResolvedValue(123n); - const result = await loadAndAssertAddCustomToken(validParams); + spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ + ['icrc1:name', { Text: tokenName }], + ['icrc1:symbol', { Text: tokenSymbol }], + ['icrc1:decimals', { Nat: BigInt(tokenDecimals) }], + ['icrc1:fee', { Nat: tokenFee }] + ]); - expect(result).toEqual({ result: 'error' }); + const result = await loadAndAssertAddCustomToken({ + ...validParams, + icrcTokens: [existingToken] + }); - expect(spyToastsError).toHaveBeenNthCalledWith(1, { - msg: { text: get(i18n).tokens.import.error.no_metadata } + expect(result).toEqual({ result: 'error' }); + + expect(spyToastsError).toHaveBeenNthCalledWith(1, { + msg: { text: get(i18n).tokens.error.duplicate_metadata } + }); }); }); - it('should return error if token already exits', async () => { - spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( - Principal.fromText(mockLedgerCanisterId) - ); + describe('with index canister', () => { + it('should return error if ledger is not related to index', async () => { + indexCanisterMock.ledgerId.mockResolvedValue( + Principal.fromText('2ouva-viaaa-aaaaq-aaamq-cai') + ); + + const result = await loadAndAssertAddCustomToken({ + ...validParams, + indexCanisterId: mockIndexCanisterId + }); - spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ - balance: 100n, - transactions: [], - oldest_tx_id: [0n] + expect(result).toEqual({ result: 'error' }); }); - spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ - ['icrc1:name', { Text: tokenName }], - ['icrc1:symbol', { Text: tokenSymbol }], - ['icrc1:decimals', { Nat: BigInt(tokenDecimals) }], - ['icrc1:fee', { Nat: tokenFee }] - ]); + it('should return error if metadata are undefined', async () => { + spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( + Principal.fromText(mockLedgerCanisterId) + ); - const result = await loadAndAssertAddCustomToken({ - ...validParams, - icrcTokens: [existingToken] - }); + spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ + balance: 100n, + transactions: [], + oldest_tx_id: [0n] + }); - expect(result).toEqual({ result: 'error' }); + spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([]); - expect(spyToastsError).toHaveBeenNthCalledWith(1, { - msg: { text: get(i18n).tokens.error.duplicate_metadata } + const result = await loadAndAssertAddCustomToken({ + ...validParams, + indexCanisterId: mockIndexCanisterId + }); + + expect(result).toEqual({ result: 'error' }); + + expect(spyToastsError).toHaveBeenNthCalledWith(1, { + msg: { text: get(i18n).tokens.import.error.no_metadata } + }); + }); + + it('should return error if token already exits', async () => { + spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( + Principal.fromText(mockLedgerCanisterId) + ); + + spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ + balance: 100n, + transactions: [], + oldest_tx_id: [0n] + }); + + spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ + ['icrc1:name', { Text: tokenName }], + ['icrc1:symbol', { Text: tokenSymbol }], + ['icrc1:decimals', { Nat: BigInt(tokenDecimals) }], + ['icrc1:fee', { Nat: tokenFee }] + ]); + + const result = await loadAndAssertAddCustomToken({ + ...validParams, + indexCanisterId: mockIndexCanisterId, + icrcTokens: [existingToken] + }); + + expect(result).toEqual({ result: 'error' }); + + expect(spyToastsError).toHaveBeenNthCalledWith(1, { + msg: { text: get(i18n).tokens.error.duplicate_metadata } + }); }); }); }); describe('success', () => { beforeEach(() => { - spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( - Principal.fromText(mockLedgerCanisterId) - ); - - spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ - balance: 100n, - transactions: [], - oldest_tx_id: [0n] - }); - spyMetadata = ledgerCanisterMock.metadata.mockResolvedValue([ ['icrc1:name', { Text: tokenName }], ['icrc1:symbol', { Text: tokenSymbol }], @@ -223,6 +247,49 @@ describe('ic-add-custom-tokens.service', () => { ]); }); + const expectedBalance = 100n; + + type LoadAndAssertAddCustomTokenParams = Partial & { + identity: OptionIdentity; + icrcTokens: IcToken[]; + }; + + const assertUpdateCallMetadata = async (params: LoadAndAssertAddCustomTokenParams) => { + await loadAndAssertAddCustomToken(params); + + expect(spyMetadata).toHaveBeenNthCalledWith(1, { + certified: true + }); + }; + + const assertLoadToken = async (params: LoadAndAssertAddCustomTokenParams) => { + const result = await loadAndAssertAddCustomToken(params); + + expect(result.result).toBe('success'); + expect(result.data).toBeDefined(); + expect(result.data?.balance).toBe(expectedBalance); + expect(result.data?.token).toMatchObject({ + name: tokenName, + symbol: tokenSymbol, + decimals: tokenDecimals + }); + }; + + const assertLoadTokenDifferent = async (params: LoadAndAssertAddCustomTokenParams) => { + const { result } = await loadAndAssertAddCustomToken({ + ...params, + icrcTokens: [ + { + ...existingToken, + name: 'Another name', + symbol: 'Another symbol' + } + ] + }); + + expect(result).toBe('success'); + }; + it('should init ledger with expected canister id', async () => { await loadAndAssertAddCustomToken(validParams); @@ -232,68 +299,100 @@ describe('ic-add-custom-tokens.service', () => { ); }); - it('should init index with expected canister id', async () => { - await loadAndAssertAddCustomToken(validParams); + describe('without index canister', () => { + beforeEach(() => { + spyBalance = ledgerCanisterMock.balance.mockResolvedValue(expectedBalance); + }); - expect(spyIndexCreate).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ canisterId: Principal.fromText(mockIndexCanisterId) }) - ); - }); + it('should accept loading without indexCanisterId', async () => { + const { result } = await loadAndAssertAddCustomToken({ + identity: mockIdentity, + icrcTokens: [], + ledgerCanisterId: mockLedgerCanisterId + }); - it('should call with an update ledgerId to ensure Index and Ledger are related', async () => { - await loadAndAssertAddCustomToken(validParams); + expect(result).toBe('success'); + }); - expect(spyLedgerId).toHaveBeenNthCalledWith(1, { - certified: true + it('should call with an update balance to retrieve the current balance of the token', async () => { + await loadAndAssertAddCustomToken(validParams); + + expect(spyBalance).toHaveBeenNthCalledWith(1, { + certified: true, + owner: mockPrincipal + }); }); - }); - it('should call with an update getTransactions to retrieve the current balance of the token', async () => { - await loadAndAssertAddCustomToken(validParams); + it('should call with an update metadata to retrieve the details of the token', async () => { + await assertUpdateCallMetadata(validParams); + }); - expect(spyGetTransactions).toHaveBeenNthCalledWith(1, { - account: getIcrcAccount(mockIdentity.getPrincipal()), - certified: true, - max_results: 0n, - start: undefined + it('should successfully load a new token', async () => { + await assertLoadToken(validParams); + }); + + it('should successfully load a new token if name and symbol is different', async () => { + await assertLoadTokenDifferent(validParams); }); }); - it('should call with an update metadata to retrieve the details of the token', async () => { - await loadAndAssertAddCustomToken(validParams); + describe('with index canister', () => { + const validParamsWithIndex = { + ...validParams, + indexCanisterId: mockIndexCanisterId + }; + + beforeEach(() => { + spyLedgerId = indexCanisterMock.ledgerId.mockResolvedValue( + Principal.fromText(mockLedgerCanisterId) + ); + + spyGetTransactions = indexCanisterMock.getTransactions.mockResolvedValue({ + balance: expectedBalance, + transactions: [], + oldest_tx_id: [0n] + }); + }); - expect(spyMetadata).toHaveBeenNthCalledWith(1, { - certified: true + it('should init index with expected canister id', async () => { + await loadAndAssertAddCustomToken(validParamsWithIndex); + + expect(spyIndexCreate).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ canisterId: Principal.fromText(mockIndexCanisterId) }) + ); }); - }); - it('should successfully load a new token', async () => { - const result = await loadAndAssertAddCustomToken(validParams); + it('should call with an update ledgerId to ensure Index and Ledger are related', async () => { + await loadAndAssertAddCustomToken(validParamsWithIndex); - expect(result.result).toBe('success'); - expect(result.data).toBeDefined(); - expect(result.data?.balance).toBe(100n); - expect(result.data?.token).toMatchObject({ - name: 'Test Token', - symbol: 'TEST', - decimals: 8 + expect(spyLedgerId).toHaveBeenNthCalledWith(1, { + certified: true + }); }); - }); - it('should successfully load a new token if name and symbol is different', async () => { - const { result } = await loadAndAssertAddCustomToken({ - ...validParams, - icrcTokens: [ - { - ...existingToken, - name: 'Another name', - symbol: 'Another symbol' - } - ] + it('should call with an update getTransactions to retrieve the current balance of the token', async () => { + await loadAndAssertAddCustomToken(validParamsWithIndex); + + expect(spyGetTransactions).toHaveBeenNthCalledWith(1, { + account: getIcrcAccount(mockIdentity.getPrincipal()), + certified: true, + max_results: 0n, + start: undefined + }); }); - expect(result).toBe('success'); + it('should call with an update metadata to retrieve the details of the token', async () => { + await assertUpdateCallMetadata(validParamsWithIndex); + }); + + it('should successfully load a new token', async () => { + await assertLoadToken(validParamsWithIndex); + }); + + it('should successfully load a new token if name and symbol is different', async () => { + await assertLoadTokenDifferent(validParamsWithIndex); + }); }); }); }); diff --git a/src/frontend/src/tests/icp/validation/ic-token.validation.spec.ts b/src/frontend/src/tests/icp/validation/ic-token.validation.spec.ts index a1fade1076..1225849405 100644 --- a/src/frontend/src/tests/icp/validation/ic-token.validation.spec.ts +++ b/src/frontend/src/tests/icp/validation/ic-token.validation.spec.ts @@ -1,3 +1,4 @@ +import { IC_CKBTC_INDEX_CANISTER_ID } from '$env/networks.icrc.env'; import type { IcToken } from '$icp/types/ic-token'; import { isIcCkToken, @@ -9,9 +10,13 @@ import { } from '$icp/validation/ic-token.validation'; import { mockValidIcCkToken, mockValidIcToken } from '$tests/mocks/ic-tokens.mock'; import { mockValidToken } from '$tests/mocks/tokens.mock'; -import { describe, expect, it } from 'vitest'; describe('ic-token.validation', () => { + const mockValidIcTokenWithIndex: IcToken = { + ...mockValidIcToken, + indexCanisterId: IC_CKBTC_INDEX_CANISTER_ID + }; + describe('isIcToken', () => { it('should return true for a valid IcToken', () => { expect(isIcToken(mockValidIcToken)).toBe(true); @@ -34,13 +39,12 @@ describe('ic-token.validation', () => { describe('isIcTokenCanistersStrict', () => { it('should return true for a valid IcToken with IcCanistersStrict', () => { - expect(isIcTokenCanistersStrict(mockValidIcToken)).toBe(true); + expect(isIcTokenCanistersStrict(mockValidIcTokenWithIndex)).toBe(true); }); - // TODO: test missing indexCanisterId when it becomes optional - // it('should return false for a valid IcToken without strict canisters fields', () => { - // expect(isIcTokenCanistersStrict(validIcToken)).toBe(false); - // }); + it('should return false for a valid IcToken without strict canisters fields', () => { + expect(isIcTokenCanistersStrict(mockValidIcToken)).toBe(false); + }); it('should return false for a token type casted to IcToken', () => { expect(isIcTokenCanistersStrict(mockValidToken as IcToken)).toBe(false); @@ -49,13 +53,12 @@ describe('ic-token.validation', () => { describe('isNotIcTokenCanistersStrict', () => { it('should return false for a valid IcToken with IcCanistersStrict', () => { - expect(isNotIcTokenCanistersStrict(mockValidIcToken)).toBe(false); + expect(isNotIcTokenCanistersStrict(mockValidIcTokenWithIndex)).toBe(false); }); - // TODO: test missing indexCanisterId when it becomes optional - // it('should return true for a valid IcToken without strict canisters fields', () => { - // expect(isNotIcTokenCanistersStrict(validIcToken)).toBe(true); - // }); + it('should return true for a valid IcToken without strict canisters fields', () => { + expect(isNotIcTokenCanistersStrict(mockValidIcToken)).toBe(true); + }); it('should return true for a token type casted to IcToken', () => { expect(isNotIcTokenCanistersStrict(mockValidToken as IcToken)).toBe(true); diff --git a/src/frontend/src/tests/mocks/ic-tokens.mock.ts b/src/frontend/src/tests/mocks/ic-tokens.mock.ts index 618d26264e..2e5a7b4562 100644 --- a/src/frontend/src/tests/mocks/ic-tokens.mock.ts +++ b/src/frontend/src/tests/mocks/ic-tokens.mock.ts @@ -1,11 +1,9 @@ -import { IC_CKBTC_INDEX_CANISTER_ID, IC_CKBTC_LEDGER_CANISTER_ID } from '$env/networks.icrc.env'; +import { IC_CKBTC_LEDGER_CANISTER_ID } from '$env/networks.icrc.env'; import type { IcCanisters, IcCkToken, IcToken } from '$icp/types/ic-token'; import { mockValidToken } from '$tests/mocks/tokens.mock'; export const mockValidIcCanisters: IcCanisters = { - ledgerCanisterId: IC_CKBTC_LEDGER_CANISTER_ID, - // TODO: to be removed when indexCanisterId becomes optional - indexCanisterId: IC_CKBTC_INDEX_CANISTER_ID + ledgerCanisterId: IC_CKBTC_LEDGER_CANISTER_ID }; export const mockValidIcToken: IcToken = {