Skip to content

Commit

Permalink
Recover from interrupted canister creation in frontend (#6132)
Browse files Browse the repository at this point in the history
# Motivation

Currently, the NNS dapp backend canister monitors every transaction on
the ICP ledger in order to catch canister creation funding transactions
for which the notify+attach process might have been interrupted.
We want to remove this logic from the backend and perform it in the
frontend in order to simplify the responsibilities of the backend
canister.

In this PR we check if loaded transactions contain any canister creation
funding transactions for which there is no corresponding canister.
This is done if the user visits the wallet page after visiting the
canisters page.
Since it should be rare for canister creation to be interrupted, we
assume that when it happens, a user will look around for their canister
and eventually trigger the recovery process.

# Changes

1. In `NnsWallet`, if the canisters are loaded, check if we need to
notify for interrupted canister creation.

# Tests

1. Unit tests added.
2. Tested manually with a backend canister where the fallback process is
disabled.
3. Tested manually with a backend canister that still has the fallback
process as well to make sure this doesn't break anything until it's
remove from the backend.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Jan 9, 2025
1 parent 58b8d34 commit 4468b11
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ proposal is successful, the changes it released will be moved from this file to
#### Changed

- Allow `dfinity:` token prefix in QR code for ICP payment.
- Recover from interrupted canister creation in the frontend.

#### Deprecated

Expand Down
32 changes: 32 additions & 0 deletions frontend/src/lib/pages/NnsWallet.svelte
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<script lang="ts">
import { goto } from "$app/navigation";
import IC_LOGO from "$lib/assets/icp.svg";
import type { CanisterDetails } from "$lib/canisters/nns-dapp/nns-dapp.types";
import HardwareWalletListNeuronsButton from "$lib/components/accounts/HardwareWalletListNeuronsButton.svelte";
import HardwareWalletShowActionButton from "$lib/components/accounts/HardwareWalletShowActionButton.svelte";
import LedgerNeuronHotkeyWarning from "$lib/components/accounts/LedgerNeuronHotkeyWarning.svelte";
Expand Down Expand Up @@ -28,6 +29,7 @@
import { createSwapCanisterAccountsStore } from "$lib/derived/sns-swap-canisters-accounts.derived";
import IcpTransactionModal from "$lib/modals/accounts/IcpTransactionModal.svelte";
import WalletModals from "$lib/modals/accounts/WalletModals.svelte";
import { notifyAndAttachCanisterIfNeeded } from "$lib/services/canisters.services";
import {
cancelPollAccounts,
loadBalance,
Expand All @@ -42,6 +44,7 @@
listNeurons,
} from "$lib/services/neurons.services";
import { authStore } from "$lib/stores/auth.store";
import { canistersStore } from "$lib/stores/canisters.store";
import { ENABLE_PERIODIC_FOLLOWING_CONFIRMATION } from "$lib/stores/feature-flags.store";
import { i18n } from "$lib/stores/i18n";
import { icpAccountBalancesStore } from "$lib/stores/icp-account-balances.store";
Expand Down Expand Up @@ -158,6 +161,35 @@
neuronAccounts: $neuronAccountsStore,
});
const tryNotifyAndAttachCanister = ({
account,
transactionsStore,
canisters,
}: {
account: Account | undefined;
transactionsStore: IcpTransactionsStoreData;
canisters: CanisterDetails[] | undefined;
}) => {
if (
isNullish(canisters) ||
isNullish(account) ||
isNullish(transactionsStore[account.identifier])
) {
return;
}
const transactions = transactionsStore[account.identifier].transactions;
notifyAndAttachCanisterIfNeeded({
transactions,
canisters,
});
};
$: tryNotifyAndAttachCanister({
account: $selectedAccountStore.account,
transactionsStore: $icpTransactionsStore,
canisters: $canistersStore?.canisters,
});
let uiTransactions: UiTransaction[] | undefined;
$: uiTransactions = makeUiTransactions({
account: $selectedAccountStore.account,
Expand Down
91 changes: 91 additions & 0 deletions frontend/src/tests/lib/pages/NnsWallet.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { resetNeuronsApiService } from "$lib/api-services/governance.api-service";
import * as canistersApi from "$lib/api/canisters.api";
import * as governanceApi from "$lib/api/governance.api";
import * as indexApi from "$lib/api/icp-index.api";
import * as ledgerApi from "$lib/api/icp-ledger.api";
Expand All @@ -14,16 +15,19 @@ import { AppPath } from "$lib/constants/routes.constants";
import { pageStore } from "$lib/derived/page.derived";
import NnsWallet from "$lib/pages/NnsWallet.svelte";
import { cancelPollAccounts } from "$lib/services/icp-accounts.services";
import { canistersStore } from "$lib/stores/canisters.store";
import { overrideFeatureFlagsStore } from "$lib/stores/feature-flags.store";
import { icpTransactionsStore } from "$lib/stores/icp-transactions.store";
import { neuronsStore } from "$lib/stores/neurons.store";
import { getCanisterCreationCmcAccountIdentifierHex } from "$lib/utils/canisters.utils";
import { getSwapCanisterAccount } from "$lib/utils/sns.utils";
import { page } from "$mocks/$app/stores";
import {
mockIdentity,
resetIdentity,
setNoIdentity,
} from "$tests/mocks/auth.store.mock";
import { mockCanister } from "$tests/mocks/canisters.mock";
import {
mockAccountDetails,
mockAccountsStoreData,
Expand Down Expand Up @@ -919,6 +923,93 @@ describe("NnsWallet", () => {
expect(spyClaimNeuron).toBeCalledTimes(0);
});
});

describe("when there are canister creation transactions", () => {
const createCanisterMemo = 0x41455243n;
const cmcAccountIdentifierHex =
getCanisterCreationCmcAccountIdentifierHex({
controller: mockIdentity.getPrincipal(),
});
const fundingTransaction = createMockSendTransactionWithId({
to: cmcAccountIdentifierHex,
memo: createCanisterMemo,
});

it("should notify for missing canisters", async () => {
vi.spyOn(canistersApi, "notifyAndAttachCanister").mockResolvedValue(
undefined
);
vi.spyOn(indexApi, "getTransactions").mockResolvedValue({
transactions: [fundingTransaction],
oldestTxId,
balance: mainBalanceE8s,
});

canistersStore.setCanisters({
certified: true,
canisters: [],
});

await renderWallet({
accountIdentifier: mockMainAccount.identifier,
});

expect(canistersApi.notifyAndAttachCanister).toBeCalledWith({
blockIndex: fundingTransaction.id,
identity: mockIdentity,
});
expect(canistersApi.notifyAndAttachCanister).toBeCalledTimes(1);
});

it("should not notify for present canisters", async () => {
vi.spyOn(canistersApi, "notifyAndAttachCanister").mockResolvedValue(
undefined
);
vi.spyOn(indexApi, "getTransactions").mockResolvedValue({
transactions: [fundingTransaction],
oldestTxId,
balance: mainBalanceE8s,
});

canistersStore.setCanisters({
certified: true,
canisters: [
{
...mockCanister,
block_index: [fundingTransaction.id],
},
],
});

await renderWallet({
accountIdentifier: mockMainAccount.identifier,
});

expect(canistersApi.notifyAndAttachCanister).toBeCalledTimes(0);
});

it("should not notify if canisters aren't loaded", async () => {
vi.spyOn(canistersApi, "notifyAndAttachCanister").mockResolvedValue(
undefined
);
vi.spyOn(indexApi, "getTransactions").mockResolvedValue({
transactions: [fundingTransaction],
oldestTxId,
balance: mainBalanceE8s,
});

canistersStore.setCanisters({
certified: undefined,
canisters: undefined,
});

await renderWallet({
accountIdentifier: mockMainAccount.identifier,
});

expect(canistersApi.notifyAndAttachCanister).toBeCalledTimes(0);
});
});
});

describe("accounts loaded (Subaccount)", () => {
Expand Down

0 comments on commit 4468b11

Please sign in to comment.