From 8509c8c515a2322d9d5ecab815855d5a8738185d Mon Sep 17 00:00:00 2001 From: David de Kloet <122978264+dskloetd@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:46:16 +0100 Subject: [PATCH] Don't build wrappers for aborted SNSes (#5742) # Motivation We were seeing errors in NNS dapp because it tried to call `get_buyer_state` on the swap canister of an aborted swap while that canister was out of cycles. But we should not be trying to call anything on aborted swaps because we don't care about aborted swaps. Multiple things were happening. 1. We load aggregator data into `snsAggregatorIncludingAbortedProjectsStore`, [here](https://github.com/dfinity/nns-dapp/blob/94df88c1e01a9677ccbd760f6483edde53c6c11f/frontend/src/lib/services/public/sns.services.ts#L51C5-L51C47). From that store, we derive [snsAggregatorStore](https://github.com/dfinity/nns-dapp/blob/94df88c1e01a9677ccbd760f6483edde53c6c11f/frontend/src/lib/stores/sns-aggregator.store.ts#L34), which has aborted SNS filtered out. 2. But on the Launchpad, when we try to load the swap commitments of the user, we don't use the list of SNSes from the aggregator. We call [loadSnsWrappers](https://github.com/dfinity/nns-dapp/blob/94df88c1e01a9677ccbd760f6483edde53c6c11f/frontend/src/lib/api/sns-wrapper.api.ts#L157), which gets the list from SNSes from the SNS-W and then from rootCanisterId, it gets the other canister IDs. This will fail for 8 of the root canisters because they no longer exist. We detect these failures and ignore those SNSes. The successful SNSes are then cached. 3. But when the aggregator data is loaded, we [populate this same cache](https://github.com/dfinity/nns-dapp/blob/94df88c1e01a9677ccbd760f6483edde53c6c11f/frontend/src/lib/services/public/sns.services.ts#L33-L42) with wrappers, without having to get canister IDs from the root canisters, but before filtering out the aborted SNSes. 4. So if after this, we again try to get swap commitments, the aborted SNSes are now included in the wrappers cache and then we try to call `get_buyer_state` on them. In this PR we just skip the aborted SNSes when populating the wrappers cache from aggregator data. This makes the error go away. But ideally we only construct wrappers from aggregator data and stop calling SNS-W and root canisters altogether. That's for future PRs. # Changes 1. Skip aborted SNSes when creating SNS wrappers from aggregator data. # Tests 1. Unit test added. 2. Tested against mainnet before the offending canister was topped up, which also made the error go away. # Todos - [x] Add entry to changelog (if necessary). --- CHANGELOG-Nns-Dapp-unreleased.md | 2 + .../src/lib/services/public/sns.services.ts | 6 ++- .../lib/services/public/sns.services.spec.ts | 47 ++++++++++++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index 6bf1e21861a..560fada2654 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -29,6 +29,8 @@ proposal is successful, the changes it released will be moved from this file to #### Fixed +* Stop trying to get swap commitments from aborted SNSes. + #### Security #### Not Published diff --git a/frontend/src/lib/services/public/sns.services.ts b/frontend/src/lib/services/public/sns.services.ts index e961eb95a5e..2241e3b7019 100644 --- a/frontend/src/lib/services/public/sns.services.ts +++ b/frontend/src/lib/services/public/sns.services.ts @@ -10,6 +10,7 @@ import { isLastCall } from "$lib/utils/env.utils"; import { toToastError } from "$lib/utils/error.utils"; import { ProposalStatus, Topic, type ProposalInfo } from "@dfinity/nns"; import { Principal } from "@dfinity/principal"; +import { SnsSwapLifecycle } from "@dfinity/sns"; import { getCurrentIdentity } from "../auth.services"; export const loadSnsProjects = async (): Promise => { @@ -19,7 +20,7 @@ export const loadSnsProjects = async (): Promise => { // We load the wrappers to avoid making calls to SNS-W and Root canister for each project. // The SNS Aggregator gives us the canister ids of the SNS projects. await Promise.all( - aggregatorData.map(async ({ canister_ids }) => { + aggregatorData.map(async ({ canister_ids, lifecycle }) => { const canisterIds = { rootCanisterId: Principal.fromText(canister_ids.root_canister_id), swapCanisterId: Principal.fromText(canister_ids.swap_canister_id), @@ -29,6 +30,9 @@ export const loadSnsProjects = async (): Promise => { ledgerCanisterId: Principal.fromText(canister_ids.ledger_canister_id), indexCanisterId: Principal.fromText(canister_ids.index_canister_id), }; + if (lifecycle.lifecycle === SnsSwapLifecycle.Aborted) { + return; + } // Build certified and uncertified wrappers because SNS aggregator gives certified data. await buildAndStoreWrapper({ identity, diff --git a/frontend/src/tests/lib/services/public/sns.services.spec.ts b/frontend/src/tests/lib/services/public/sns.services.spec.ts index fc06e2e1df8..3cdf3e03313 100644 --- a/frontend/src/tests/lib/services/public/sns.services.spec.ts +++ b/frontend/src/tests/lib/services/public/sns.services.spec.ts @@ -1,7 +1,11 @@ import { clearSnsAggregatorCache } from "$lib/api-services/sns-aggregator.api-service"; import * as agent from "$lib/api/agent.api"; import * as aggregatorApi from "$lib/api/sns-aggregator.api"; -import { clearWrapperCache, wrapper } from "$lib/api/sns-wrapper.api"; +import { + clearWrapperCache, + wrappers as getWrappers, + wrapper, +} from "$lib/api/sns-wrapper.api"; import { snsFunctionsStore } from "$lib/derived/sns-functions.derived"; import { snsTotalTokenSupplyStore } from "$lib/derived/sns-total-token-supply.derived"; import { loadSnsProjects } from "$lib/services/public/sns.services"; @@ -26,6 +30,7 @@ import { } from "$tests/mocks/sns.api.mock"; import { blockAllCallsTo } from "$tests/utils/module.test-utils"; import type { HttpAgent } from "@dfinity/agent"; +import { SnsSwapLifecycle } from "@dfinity/sns"; import { get } from "svelte/store"; import { mock } from "vitest-mock-extended"; @@ -164,5 +169,45 @@ describe("SNS public services", () => { expect(data).not.toBeUndefined(); expect(data?.totalSupply).toEqual(BigInt(totalSupply)); }); + + it("should build and store wrappers, only for non-aborted SNSes", async () => { + const committedSns1 = aggregatorSnsMockWith({ + rootCanisterId: principal(0).toText(), + lifecycle: SnsSwapLifecycle.Committed, + }); + const committedSns2 = aggregatorSnsMockWith({ + rootCanisterId: principal(1).toText(), + lifecycle: SnsSwapLifecycle.Committed, + }); + const abortedSns1 = aggregatorSnsMockWith({ + rootCanisterId: principal(2).toText(), + lifecycle: SnsSwapLifecycle.Aborted, + }); + const abortedSns2 = aggregatorSnsMockWith({ + rootCanisterId: principal(3).toText(), + lifecycle: SnsSwapLifecycle.Aborted, + }); + + vi.spyOn(aggregatorApi, "querySnsProjects").mockResolvedValue([ + committedSns1, + abortedSns1, + committedSns2, + abortedSns2, + ]); + + await loadSnsProjects(); + + const wrappers = await getWrappers({ + identity: mockIdentity, + certified: true, + }); + expect(wrappers).toHaveLength(2); + expect(wrappers.has(committedSns1.canister_ids.root_canister_id)).toBe( + true + ); + expect(wrappers.has(committedSns2.canister_ids.root_canister_id)).toBe( + true + ); + }); }); });