diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index 2f5f7a2595e..7914e303525 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -38,6 +38,10 @@ proposal is successful, the changes it released will be moved from this file to #### Security +- The Portfolio, Tokens and Staking pages now confirm every token balance with + a certified call. Before, they showed the answer of one replica and never + checked it. + #### Not Published ### Operations diff --git a/frontend/src/lib/services/accounts-balances.services.ts b/frontend/src/lib/services/accounts-balances.services.ts index 966c1410558..ca44d47963d 100644 --- a/frontend/src/lib/services/accounts-balances.services.ts +++ b/frontend/src/lib/services/accounts-balances.services.ts @@ -1,5 +1,5 @@ -import { uncertifiedLoadSnsesAccountsBalances } from "$lib/services/sns-accounts-balance.services"; -import { uncertifiedLoadAccountsBalance } from "$lib/services/wallet-uncertified-accounts.services"; +import { syncIcrcAccountsBalances } from "$lib/services/icrc-accounts-balance.services"; +import { syncSnsAccountsBalances } from "$lib/services/sns-accounts-balance.services"; import type { UniverseCanisterIdText } from "$lib/types/universe"; import type { CanisterIdString } from "@icp-sdk/canisters/nns"; import { Principal } from "@icp-sdk/core/principal"; @@ -23,7 +23,7 @@ export const loadSnsAccountsBalances = async ( if (notLoadedIds.length === 0) return; - await uncertifiedLoadSnsesAccountsBalances({ + await syncSnsAccountsBalances({ rootCanisterIds: notLoadedIds.map((id) => Principal.fromText(id)), }); }; @@ -35,7 +35,7 @@ export const loadAccountsBalances = async ( if (notLoadedIds.length === 0) return; - await uncertifiedLoadAccountsBalance({ + await syncIcrcAccountsBalances({ universeIds: notLoadedIds, }); }; diff --git a/frontend/src/lib/services/icrc-accounts-balance.services.ts b/frontend/src/lib/services/icrc-accounts-balance.services.ts new file mode 100644 index 00000000000..3f953ddecca --- /dev/null +++ b/frontend/src/lib/services/icrc-accounts-balance.services.ts @@ -0,0 +1,33 @@ +import { syncAccounts } from "$lib/services/icrc-accounts.services"; +import { toastsError } from "$lib/stores/toasts.store"; +import type { UniverseCanisterIdText } from "$lib/types/universe"; +import { Principal } from "@icp-sdk/core/principal"; + +/** + * Load Icrc accounts balances and token. + * + * The query answer shows first and the certified answer replaces it. If the + * certified answer arrives first, the query answer is skipped. + * + * @param {universeIds: UniverseCanisterIdText[]} params + * @param {UniverseCanisterIdText[]} params.universeIds The Icrc environment for which the balances should be loaded. + */ +export const syncIcrcAccountsBalances = async ({ + universeIds, +}: { + universeIds: UniverseCanisterIdText[]; +}): Promise => { + const results = await Promise.allSettled( + universeIds.map((universeId) => + syncAccounts({ + ledgerCanisterId: Principal.fromText(universeId), + }) + ) + ); + + const error: boolean = + results.find(({ status }) => status === "rejected") !== undefined; + if (error) { + toastsError({ labelKey: "error.accounts_load" }); + } +}; diff --git a/frontend/src/lib/services/sns-accounts-balance.services.ts b/frontend/src/lib/services/sns-accounts-balance.services.ts index 39cecd20a23..c47ac550fd1 100644 --- a/frontend/src/lib/services/sns-accounts-balance.services.ts +++ b/frontend/src/lib/services/sns-accounts-balance.services.ts @@ -5,12 +5,13 @@ import type { RootCanisterId } from "$lib/types/sns"; /** * Load Sns projects accounts balances. * - * ⚠️ WARNING: this feature only performs "query" calls. Effective "update" is performed when a Sns project is manually selected either through the token navigation switcher or accessed directly via the browser url. + * The query answer shows first and the certified answer replaces it. If the + * certified answer arrives first, the query answer is skipped. * - * @param {rootCanisterIds: RootCanisterId[], excludeRootCanisterIds?: RootCanisterIdText[]} params + * @param {rootCanisterIds: RootCanisterId[]} params * @param {RootCanisterId[]} params.rootCanisterIds The list of root canister ids - Sns projects - for which the balance of the accounts should be fetched. */ -export const uncertifiedLoadSnsesAccountsBalances = async ({ +export const syncSnsAccountsBalances = async ({ rootCanisterIds, }: { rootCanisterIds: RootCanisterId[]; @@ -19,7 +20,6 @@ export const uncertifiedLoadSnsesAccountsBalances = async ({ rootCanisterIds.map((rootCanisterId) => loadSnsAccounts({ rootCanisterId, - strategy: "query", }) ) ); diff --git a/frontend/src/lib/services/wallet-uncertified-accounts.services.ts b/frontend/src/lib/services/wallet-uncertified-accounts.services.ts deleted file mode 100644 index 2663808e68c..00000000000 --- a/frontend/src/lib/services/wallet-uncertified-accounts.services.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { - loadAccounts, - loadIcrcToken, -} from "$lib/services/icrc-accounts.services"; -import { toastsError } from "$lib/stores/toasts.store"; -import type { UniverseCanisterIdText } from "$lib/types/universe"; -import { Principal } from "@icp-sdk/core/principal"; - -/** - * Load Icrc accounts balances and token - * - * ⚠️ WARNING: this feature only performs "query" calls. Effective "update" is performed when the universe is manually selected either through the token navigation switcher or accessed directly via the browser url. - * - * @param {universeIds: UniverseCanisterId[]; excludeUniverseIds: RootCanisterIdText[] | undefined} params - * @param {UniverseCanisterId[]} params.universeIds The Icrc environment for which the balances should be loaded. - */ -export const uncertifiedLoadAccountsBalance = async ({ - universeIds, -}: { - universeIds: UniverseCanisterIdText[]; -}): Promise => { - const results: PromiseSettledResult<[void, void]>[] = - await Promise.allSettled( - universeIds.map((universeId) => - Promise.all([ - loadAccounts({ - strategy: "query", - ledgerCanisterId: Principal.fromText(universeId), - }), - loadIcrcToken({ - ledgerCanisterId: Principal.fromText(universeId), - certified: false, - }), - ]) - ) - ); - - const error: boolean = - results.find(({ status }) => status === "rejected") !== undefined; - if (error) { - toastsError({ labelKey: "error.sns_accounts_balance_load" }); - } -}; diff --git a/frontend/src/routes/(app)/(nns)/tokens/+page.svelte b/frontend/src/routes/(app)/(nns)/tokens/+page.svelte index a868b38d98c..5feeffa57b7 100644 --- a/frontend/src/routes/(app)/(nns)/tokens/+page.svelte +++ b/frontend/src/routes/(app)/(nns)/tokens/+page.svelte @@ -26,8 +26,8 @@ import { loadCkBTCTokens } from "$lib/services/ckbtc-tokens.services"; import { loadIcpSwapTickers } from "$lib/services/icp-swap.services"; import { removeImportedTokens } from "$lib/services/imported-tokens.services"; - import { uncertifiedLoadSnsesAccountsBalances } from "$lib/services/sns-accounts-balance.services"; - import { uncertifiedLoadAccountsBalance } from "$lib/services/wallet-uncertified-accounts.services"; + import { syncSnsAccountsBalances } from "$lib/services/sns-accounts-balance.services"; + import { syncIcrcAccountsBalances } from "$lib/services/icrc-accounts-balance.services"; import { selectableUniversesStore } from "$lib/derived/selectable-universes.derived"; import { importedTokensStore } from "$lib/stores/imported-tokens.store"; import type { Account } from "$lib/types/account"; @@ -81,7 +81,7 @@ return; } - await uncertifiedLoadSnsesAccountsBalances({ + await syncSnsAccountsBalances({ rootCanisterIds: notLoadedCanisterIds.map((id) => Principal.fromText(id)), }); }; @@ -143,7 +143,7 @@ return; } - await uncertifiedLoadAccountsBalance({ + await syncIcrcAccountsBalances({ universeIds, }); }; @@ -153,7 +153,7 @@ ({ rootCanisterId }) => rootCanisterId.toText() === universeId.toText() ); if (isSnsProject) { - return uncertifiedLoadSnsesAccountsBalances({ + return syncSnsAccountsBalances({ rootCanisterIds: [universeId], }); } diff --git a/frontend/src/tests/e2e/portfolio-certified-balances.spec.ts b/frontend/src/tests/e2e/portfolio-certified-balances.spec.ts new file mode 100644 index 00000000000..96b191af34a --- /dev/null +++ b/frontend/src/tests/e2e/portfolio-certified-balances.spec.ts @@ -0,0 +1,90 @@ +import { AppPo } from "$tests/page-objects/App.page-object"; +import { PlaywrightPageObjectElement } from "$tests/page-objects/playwright.page-object"; +import { signInWithNewUser, step } from "$tests/utils/e2e.test-utils"; +import { expect, test, type Request } from "@playwright/test"; + +// The Portfolio page must confirm every token balance with a certified update +// call. Before the fix it loaded every SNS and ck token balance with a query +// call only, so one malicious replica could show a forged balance. +// +// The IC HTTP interface uses one path per call type: +// /api/v2/canister//query -> uncertified query call +// /api/v3/canister//call -> certified update call +// +// The CBOR request body carries the method name as a plain text string, so the +// test selects the balance calls by searching the raw body for +// "icrc1_balance_of". No CBOR parser is needed. + +const CALL_PATH_PATTERN = /^\/api\/v\d+\/canister\/([^/]+)\/(query|call)$/; + +const BALANCE_METHOD = "icrc1_balance_of"; + +test("Portfolio balances are confirmed with certified update calls", async ({ + page, + context, +}) => { + const queriedLedgers = new Set(); + const updatedLedgers = new Set(); + + page.on("request", (request: Request) => { + if (request.method() !== "POST") { + return; + } + + const match = CALL_PATH_PATTERN.exec(new URL(request.url()).pathname); + if (match === null) { + return; + } + + const body = request.postDataBuffer(); + if (body === null) { + return; + } + if (!body.toString("latin1").includes(BALANCE_METHOD)) { + return; + } + + const [, canisterId, callType] = match; + if (callType === "query") { + queriedLedgers.add(canisterId); + } else { + updatedLedgers.add(canisterId); + } + }); + + await page.goto("/"); + await expect(page).toHaveTitle("Portfolio | Network Nervous System"); + + await signInWithNewUser({ page, context }); + + const pageElement = PlaywrightPageObjectElement.fromPage(page); + const appPo = new AppPo(pageElement); + const portfolioPagePo = appPo.getPortfolioPo().getPortfolioPagePo(); + + await step("Wait for the Portfolio page to load the balances"); + await portfolioPagePo.getTotalAssetsCardPo().waitForLoaded(); + + await step("The Portfolio page reads at least one ledger balance"); + await expect + .poll(() => queriedLedgers.size, { timeout: 60_000 }) + .toBeGreaterThan(0); + + await step( + "Every ledger that got a balance query also gets a balance update" + ); + // On main this list keeps every queried ledger, because the Portfolio page + // sends no update call at all. + await expect + .poll( + () => [...queriedLedgers].filter((id) => !updatedLedgers.has(id)).sort(), + { timeout: 60_000 } + ) + .toEqual([]); + + await step("No ledger gets an update call without a query call"); + // The query answer must still show first, so an update call alone would mean + // the page waits for the certified answer to render a balance. + expect( + [...updatedLedgers].filter((id) => !queriedLedgers.has(id)).sort() + ).toEqual([]); +}); diff --git a/frontend/src/tests/lib/services/accounts-balances.services.spec.ts b/frontend/src/tests/lib/services/accounts-balances.services.spec.ts index 403c146a3da..ffc176c251b 100644 --- a/frontend/src/tests/lib/services/accounts-balances.services.spec.ts +++ b/frontend/src/tests/lib/services/accounts-balances.services.spec.ts @@ -3,8 +3,8 @@ import { loadSnsAccountsBalances, resetBalanceLoading, } from "$lib/services/accounts-balances.services"; +import * as walletServices from "$lib/services/icrc-accounts-balance.services"; import * as snsBalanceServices from "$lib/services/sns-accounts-balance.services"; -import * as walletServices from "$lib/services/wallet-uncertified-accounts.services"; import type { CanisterIdString } from "@icp-sdk/canisters/nns"; import { Principal } from "@icp-sdk/core/principal"; @@ -12,6 +12,7 @@ vi.mock("$lib/services/icrc-accounts.services", () => { return { loadAccounts: vi.fn(), loadIcrcToken: vi.fn(), + syncAccounts: vi.fn(), }; }); @@ -28,15 +29,9 @@ describe("accounts-balances services", () => { beforeEach(() => { resetBalanceLoading(); - accountsBalanceSpy = vi.spyOn( - walletServices, - "uncertifiedLoadAccountsBalance" - ); + accountsBalanceSpy = vi.spyOn(walletServices, "syncIcrcAccountsBalances"); - snsBalancesSpy = vi.spyOn( - snsBalanceServices, - "uncertifiedLoadSnsesAccountsBalances" - ); + snsBalancesSpy = vi.spyOn(snsBalanceServices, "syncSnsAccountsBalances"); }); describe("loadSnsBalances", () => { diff --git a/frontend/src/tests/lib/services/wallet-uncertified-accounts.services.spec.ts b/frontend/src/tests/lib/services/icrc-accounts-balance.services.spec.ts similarity index 51% rename from frontend/src/tests/lib/services/wallet-uncertified-accounts.services.spec.ts rename to frontend/src/tests/lib/services/icrc-accounts-balance.services.spec.ts index 2a598213c61..87d880bd466 100644 --- a/frontend/src/tests/lib/services/wallet-uncertified-accounts.services.spec.ts +++ b/frontend/src/tests/lib/services/icrc-accounts-balance.services.spec.ts @@ -5,7 +5,8 @@ import { } from "$lib/constants/ckbtc-canister-ids.constants"; import { universesAccountsBalance } from "$lib/derived/universes-accounts-balance.derived"; import { ckBTCTokenStore } from "$lib/derived/universes-tokens.derived"; -import * as services from "$lib/services/wallet-uncertified-accounts.services"; +import * as services from "$lib/services/icrc-accounts-balance.services"; +import { icrcAccountsStore } from "$lib/stores/icrc-accounts.store"; import * as toastsStore from "$lib/stores/toasts.store"; import { toastsError } from "$lib/stores/toasts.store"; import { resetIdentity } from "$tests/mocks/auth.store.mock"; @@ -13,9 +14,10 @@ import { mockCkBTCMainAccount, mockCkBTCToken, } from "$tests/mocks/ckbtc-accounts.mock"; +import { runResolvedPromises } from "$tests/utils/timers.test-utils"; import { get } from "svelte/store"; -describe("wallet-uncertified-accounts.services", () => { +describe("icrc-accounts-balance.services", () => { beforeEach(() => { resetIdentity(); vi.spyOn(toastsStore, "toastsError"); @@ -35,7 +37,7 @@ describe("wallet-uncertified-accounts.services", () => { .spyOn(icrcLegerApi, "queryIcrcBalance") .mockResolvedValue(mockCkBTCMainAccount.balanceUlps); - await services.uncertifiedLoadAccountsBalance(params); + await services.syncIcrcAccountsBalances(params); const store = get(universesAccountsBalance); // Nns + ckBTC + ckTESTBTC @@ -46,7 +48,55 @@ describe("wallet-uncertified-accounts.services", () => { expect(spyQuery).toBeCalled(); }); - it("should call api.getToken and load token in store", async () => { + it("should call api.queryIcrcBalance with a query and an update call", async () => { + vi.spyOn(icrcLegerApi, "queryIcrcToken").mockResolvedValue(mockCkBTCToken); + + const spyQuery = vi + .spyOn(icrcLegerApi, "queryIcrcBalance") + .mockResolvedValue(mockCkBTCMainAccount.balanceUlps); + + await services.syncIcrcAccountsBalances(params); + + await runResolvedPromises(); + + for (const canisterId of [ + CKBTC_UNIVERSE_CANISTER_ID, + CKTESTBTC_UNIVERSE_CANISTER_ID, + ]) { + expect(spyQuery).toHaveBeenCalledWith( + expect.objectContaining({ canisterId, certified: false }) + ); + expect(spyQuery).toHaveBeenCalledWith( + expect.objectContaining({ canisterId, certified: true }) + ); + } + expect(spyQuery).toHaveBeenCalledTimes(4); + }); + + it("should store the certified balance and not the query answer", async () => { + const queryBalanceUlps = 1n; + const certifiedBalanceUlps = 2n; + + vi.spyOn(icrcLegerApi, "queryIcrcToken").mockResolvedValue(mockCkBTCToken); + vi.spyOn(icrcLegerApi, "queryIcrcBalance").mockImplementation( + async ({ certified }) => + certified ? certifiedBalanceUlps : queryBalanceUlps + ); + + await services.syncIcrcAccountsBalances(params); + + await runResolvedPromises(); + + const store = get(icrcAccountsStore); + expect(store[CKBTC_UNIVERSE_CANISTER_ID.toText()]).toMatchObject({ + certified: true, + }); + expect( + store[CKBTC_UNIVERSE_CANISTER_ID.toText()].accounts[0].balanceUlps + ).toEqual(certifiedBalanceUlps); + }); + + it("should call api.getToken and load the certified token in store", async () => { const spyQuery = vi .spyOn(icrcLegerApi, "queryIcrcToken") .mockResolvedValue(mockCkBTCToken); @@ -55,12 +105,14 @@ describe("wallet-uncertified-accounts.services", () => { mockCkBTCMainAccount.balanceUlps ); - await services.uncertifiedLoadAccountsBalance(params); + await services.syncIcrcAccountsBalances(params); + + await runResolvedPromises(); const store = get(ckBTCTokenStore); const token = { token: mockCkBTCToken, - certified: false, + certified: true, }; expect(store).toEqual({ [CKBTC_UNIVERSE_CANISTER_ID.toText()]: token, @@ -75,7 +127,9 @@ describe("wallet-uncertified-accounts.services", () => { vi.spyOn(icrcLegerApi, "queryIcrcToken").mockResolvedValue(mockCkBTCToken); vi.spyOn(icrcLegerApi, "queryIcrcBalance").mockRejectedValue(new Error()); - await services.uncertifiedLoadAccountsBalance(params); + await services.syncIcrcAccountsBalances(params); + + await runResolvedPromises(); expect(toastsError).toHaveBeenCalled(); }); diff --git a/frontend/src/tests/lib/services/sns-accounts-balance.services.spec.ts b/frontend/src/tests/lib/services/sns-accounts-balance.services.spec.ts index 75008490ea5..ac821116964 100644 --- a/frontend/src/tests/lib/services/sns-accounts-balance.services.spec.ts +++ b/frontend/src/tests/lib/services/sns-accounts-balance.services.spec.ts @@ -1,10 +1,12 @@ import * as ledgerApi from "$lib/api/icrc-ledger.api"; import { universesAccountsBalance } from "$lib/derived/universes-accounts-balance.derived"; import * as services from "$lib/services/sns-accounts-balance.services"; +import { icrcAccountsStore } from "$lib/stores/icrc-accounts.store"; import { resetIdentity } from "$tests/mocks/auth.store.mock"; import { mockSnsMainAccount } from "$tests/mocks/sns-accounts.mock"; import { principal } from "$tests/mocks/sns-projects.mock"; import { setSnsProjects } from "$tests/utils/sns.test-utils"; +import { runResolvedPromises } from "$tests/utils/timers.test-utils"; import { toastsStore } from "@dfinity/gix-components"; import { tick } from "svelte"; import { get } from "svelte/store"; @@ -29,7 +31,7 @@ describe("sns-accounts-balance.services", () => { .spyOn(ledgerApi, "queryIcrcBalance") .mockResolvedValue(mockSnsMainAccount.balanceUlps); - await services.uncertifiedLoadSnsesAccountsBalances({ + await services.syncSnsAccountsBalances({ rootCanisterIds: [rootCanisterId], }); @@ -44,16 +46,68 @@ describe("sns-accounts-balance.services", () => { expect(spyQuery).toBeCalled(); }); + it("should call api.queryIcrcBalance with a query and an update call", async () => { + const spyQuery = vi + .spyOn(ledgerApi, "queryIcrcBalance") + .mockResolvedValue(mockSnsMainAccount.balanceUlps); + + await services.syncSnsAccountsBalances({ + rootCanisterIds: [rootCanisterId], + }); + + await runResolvedPromises(); + + expect(spyQuery).toHaveBeenCalledWith( + expect.objectContaining({ + canisterId: ledgerCanisterId, + certified: false, + }) + ); + expect(spyQuery).toHaveBeenCalledWith( + expect.objectContaining({ + canisterId: ledgerCanisterId, + certified: true, + }) + ); + expect(spyQuery).toHaveBeenCalledTimes(2); + }); + + it("should store the certified balance and not the query answer", async () => { + const queryBalanceUlps = 1n; + const certifiedBalanceUlps = 2n; + + vi.spyOn(ledgerApi, "queryIcrcBalance").mockImplementation( + async ({ certified }) => + certified ? certifiedBalanceUlps : queryBalanceUlps + ); + + await services.syncSnsAccountsBalances({ + rootCanisterIds: [rootCanisterId], + }); + + await runResolvedPromises(); + + const store = get(icrcAccountsStore); + expect(store[ledgerCanisterId.toText()]).toMatchObject({ + certified: true, + }); + expect(store[ledgerCanisterId.toText()].accounts[0].balanceUlps).toEqual( + certifiedBalanceUlps + ); + }); + it("should toast error", async () => { vi.spyOn(console, "error").mockImplementation(() => undefined); vi.spyOn(ledgerApi, "queryIcrcBalance").mockRejectedValue(new Error()); expect(get(toastsStore)).toEqual([]); - await services.uncertifiedLoadSnsesAccountsBalances({ + await services.syncSnsAccountsBalances({ rootCanisterIds: [rootCanisterId], }); + await runResolvedPromises(); + expect(get(toastsStore)).toMatchObject([ { level: "error", diff --git a/frontend/src/tests/routes/app/portfolio/page.spec.ts b/frontend/src/tests/routes/app/portfolio/page.spec.ts index d8683a384d7..e937e23562b 100644 --- a/frontend/src/tests/routes/app/portfolio/page.spec.ts +++ b/frontend/src/tests/routes/app/portfolio/page.spec.ts @@ -231,57 +231,37 @@ describe("Portfolio route", () => { await renderPage(); - // Should be called 5 times total (2 ckBTC + 2 ICRC + 1 ImportedToken + 2 SNS) - expect(icrcLedgerApi.queryIcrcBalance).toBeCalledTimes(7); - - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(1, { - canisterId: CKBTC_UNIVERSE_CANISTER_ID, - certified: false, - identity, - account, - }); - - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(2, { - canisterId: CKTESTBTC_UNIVERSE_CANISTER_ID, - certified: false, - identity, - account, - }); - - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(3, { - canisterId: CKETH_UNIVERSE_CANISTER_ID, - certified: false, - identity, - account, - }); - - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(4, { - canisterId: CKUSDC_UNIVERSE_CANISTER_ID, - certified: false, - identity, - account, - }); - - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(5, { - canisterId: importedToken1Id, - certified: false, - identity, - account, - }); + const ledgerCanisterIds = [ + CKBTC_UNIVERSE_CANISTER_ID, + CKTESTBTC_UNIVERSE_CANISTER_ID, + CKETH_UNIVERSE_CANISTER_ID, + CKUSDC_UNIVERSE_CANISTER_ID, + importedToken1Id, + tetrisSNS.ledgerCanisterId, + doomSNS.ledgerCanisterId, + ]; + + // 7 ledgers (2 ckBTC + 2 ICRC + 1 ImportedToken + 2 SNS), each with one + // query call and one update call. + expect(icrcLedgerApi.queryIcrcBalance).toBeCalledTimes( + 2 * ledgerCanisterIds.length + ); - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(6, { - canisterId: tetrisSNS.ledgerCanisterId, - certified: false, - identity, - account, - }); + for (const canisterId of ledgerCanisterIds) { + expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenCalledWith({ + canisterId, + certified: false, + identity, + account, + }); - expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenNthCalledWith(7, { - canisterId: doomSNS.ledgerCanisterId, - certified: false, - identity, - account, - }); + expect(icrcLedgerApi.queryIcrcBalance).toHaveBeenCalledWith({ + canisterId, + certified: true, + identity, + account, + }); + } }); describe("should render the Portfolio page", async () => { @@ -419,6 +399,59 @@ describe("Portfolio route", () => { expect(await stakedTokensCardPo.getInfoRow().isPresent()).toBe(true); }); + it("should show the certified balance when it differs from the query answer", async () => { + // TODO: Move this to a helper or similar + vi.spyOn(isDesktopViewportStore, "subscribe").mockImplementation( + (fn) => { + fn(true); + return () => {}; + } + ); + + // A malicious replica answers the query call with a forged ckBTC + // balance. The certified update call answers with the real one. + const forgedCkBTCBalanceE8s = 10n * 100_000_000n; + + vi.spyOn(icrcLedgerApi, "queryIcrcBalance").mockImplementation( + async ({ canisterId, certified }) => { + if ( + !certified && + canisterId.toText() === CKBTC_UNIVERSE_CANISTER_ID.toText() + ) { + return forgedCkBTCBalanceE8s; + } + + const balancesMap = { + [CKBTC_UNIVERSE_CANISTER_ID.toText()]: ckBTCBalanceE8s, + [CKTESTBTC_UNIVERSE_CANISTER_ID.toText()]: ckBTCBalanceE8s, + [CKETH_UNIVERSE_CANISTER_ID.toText()]: ckETHBalanceUlps, + [CKUSDC_UNIVERSE_CANISTER_ID.toText()]: ckUSDCBalanceE6s, + [tetrisSNS.ledgerCanisterId.toText()]: tetrisBalanceE8s, + [doomSNS.ledgerCanisterId.toText()]: doomBalanceE8s, + [importedToken1Id.toText()]: importedToken1BalanceE6s, + }; + + return balancesMap[canisterId.toText()]; + } + ); + + const po = await renderPage(); + await runResolvedPromises(); + const portfolioPagePo = po.getPortfolioPagePo(); + + expect( + await portfolioPagePo + .getHeldRestTokensCardPo() + .getHeldTokensBalanceInNativeCurrency() + ).toEqual(["1.00 ckBTC", "1.00 ckTESTBTC", "0.10 ckETH"]); + + // The same total as with an honest query answer. The forged 10 ckBTC + // would add $900'000 to it. + expect( + await portfolioPagePo.getTotalAssetsCardPo().getPrimaryAmount() + ).toBe("$203’451"); + }); + it.skip("should not show failed SNS", async () => { // TODO: Move this to a helper or similar vi.spyOn(isDesktopViewportStore, "subscribe").mockImplementation( diff --git a/frontend/src/tests/routes/app/tokens/page.spec.ts b/frontend/src/tests/routes/app/tokens/page.spec.ts index c680dea0c0a..520514c0d84 100644 --- a/frontend/src/tests/routes/app/tokens/page.spec.ts +++ b/frontend/src/tests/routes/app/tokens/page.spec.ts @@ -1057,7 +1057,8 @@ describe("Tokens route", () => { await runResolvedPromises(); expect(icrcLedgerApi.queryIcrcBalance).toBeCalledTimes( - notFailedTokenCount + // One query call and one update call per token. + 2 * notFailedTokenCount ); // Add a failed token @@ -1070,7 +1071,8 @@ describe("Tokens route", () => { await runResolvedPromises(); expect(icrcLedgerApi.queryIcrcBalance).toBeCalledTimes( - notFailedTokenCount + // One query call and one update call per token. + 2 * notFailedTokenCount ); }); });