From 50ee4155957c696e6aa7dd9d9f79d7e8c3d57d2c Mon Sep 17 00:00:00 2001 From: David de Kloet Date: Wed, 8 Jan 2025 11:50:06 +0100 Subject: [PATCH] Set block_index when attaching created canister --- frontend/src/lib/api/canisters.api.ts | 4 ++++ .../lib/canisters/nns-dapp/nns-dapp.canister.ts | 5 ++++- .../canisters/nns-dapp/nns-dapp.certified.idl.js | 2 ++ frontend/src/lib/canisters/nns-dapp/nns-dapp.did | 6 ++++++ .../src/lib/canisters/nns-dapp/nns-dapp.idl.js | 2 ++ .../src/lib/canisters/nns-dapp/nns-dapp.types.ts | 2 ++ frontend/src/tests/lib/api/canisters.api.spec.ts | 12 +++++++++--- .../tests/lib/canisters/nns-dapp.canister.spec.ts | 14 +++++++++++++- .../src/tests/lib/pages/CanisterDetail.spec.ts | 11 ++++++++++- frontend/src/tests/mocks/canisters.mock.ts | 4 +++- 10 files changed, 55 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/api/canisters.api.ts b/frontend/src/lib/api/canisters.api.ts index 498a22ab931..bc68eea757b 100644 --- a/frontend/src/lib/api/canisters.api.ts +++ b/frontend/src/lib/api/canisters.api.ts @@ -85,6 +85,7 @@ export const queryCanisterDetails = async ({ return response; }; +// Attaches a canister that's not created by the user to the user account. export const attachCanister = async ({ identity, name, @@ -107,6 +108,8 @@ export const attachCanister = async ({ await nnsDapp.attachCanister({ name: name ?? "", canisterId, + // blockIndex is only specified for canisters created by the user. + blockIndex: undefined, }); logWithTimestamp("Attaching canister call complete."); @@ -266,6 +269,7 @@ export const createCanister = async ({ await nnsDapp.attachCanister({ name: name ?? "", canisterId, + blockIndex: blockHeight, }); } catch (error: unknown) { // If the background task finishes earlier, we might get CanisterAlreadyAttachedError. diff --git a/frontend/src/lib/canisters/nns-dapp/nns-dapp.canister.ts b/frontend/src/lib/canisters/nns-dapp/nns-dapp.canister.ts index eadecd59b60..7c1c6dc048f 100644 --- a/frontend/src/lib/canisters/nns-dapp/nns-dapp.canister.ts +++ b/frontend/src/lib/canisters/nns-dapp/nns-dapp.canister.ts @@ -2,7 +2,7 @@ import { Actor } from "@dfinity/agent"; import { AccountIdentifier } from "@dfinity/ledger-icp"; import type { ProposalId } from "@dfinity/nns"; import type { Principal } from "@dfinity/principal"; -import { nonNullish } from "@dfinity/utils"; +import { nonNullish, toNullable } from "@dfinity/utils"; import type { NNSDappCanisterOptions } from "./nns-dapp.canister.types"; import { idlFactory as certifiedIdlFactory } from "./nns-dapp.certified.idl"; import { @@ -218,13 +218,16 @@ export class NNSDappCanister { public attachCanister = async ({ name, canisterId, + blockIndex, }: { name: string; canisterId: Principal; + blockIndex: bigint | undefined; }): Promise => { const response = await this.certifiedService.attach_canister({ name, canister_id: canisterId, + block_index: toNullable(blockIndex), }); if ("Ok" in response) { return; diff --git a/frontend/src/lib/canisters/nns-dapp/nns-dapp.certified.idl.js b/frontend/src/lib/canisters/nns-dapp/nns-dapp.certified.idl.js index 7e760acffad..417336a6eb7 100644 --- a/frontend/src/lib/canisters/nns-dapp/nns-dapp.certified.idl.js +++ b/frontend/src/lib/canisters/nns-dapp/nns-dapp.certified.idl.js @@ -5,6 +5,7 @@ export const idlFactory = ({ IDL }) => { const AttachCanisterRequest = IDL.Record({ name: IDL.Text, canister_id: IDL.Principal, + block_index: IDL.Opt(IDL.Nat64), }); const AttachCanisterResponse = IDL.Variant({ Ok: IDL.Null, @@ -47,6 +48,7 @@ export const idlFactory = ({ IDL }) => { const CanisterDetails = IDL.Record({ name: IDL.Text, canister_id: IDL.Principal, + block_index: IDL.Opt(IDL.Nat64), }); const ImportedToken = IDL.Record({ index_canister_id: IDL.Opt(IDL.Principal), diff --git a/frontend/src/lib/canisters/nns-dapp/nns-dapp.did b/frontend/src/lib/canisters/nns-dapp/nns-dapp.did index 2f2113ab7b6..f64466141f1 100644 --- a/frontend/src/lib/canisters/nns-dapp/nns-dapp.did +++ b/frontend/src/lib/canisters/nns-dapp/nns-dapp.did @@ -74,12 +74,18 @@ type CanisterDetails = record { name: text; canister_id: principal; + // The `block_index` that was passed to `notify_create_canister` if the + // canister was created by the user. + block_index: opt nat64; }; type AttachCanisterRequest = record { name: text; canister_id: principal; + // The `block_index` that was passed to `notify_create_canister` if the + // canister was created by the user. + block_index: opt nat64; }; type AttachCanisterResponse = diff --git a/frontend/src/lib/canisters/nns-dapp/nns-dapp.idl.js b/frontend/src/lib/canisters/nns-dapp/nns-dapp.idl.js index c937b56dd6e..0eba256932f 100644 --- a/frontend/src/lib/canisters/nns-dapp/nns-dapp.idl.js +++ b/frontend/src/lib/canisters/nns-dapp/nns-dapp.idl.js @@ -5,6 +5,7 @@ export const idlFactory = ({ IDL }) => { const AttachCanisterRequest = IDL.Record({ name: IDL.Text, canister_id: IDL.Principal, + block_index: IDL.Opt(IDL.Nat64), }); const AttachCanisterResponse = IDL.Variant({ Ok: IDL.Null, @@ -47,6 +48,7 @@ export const idlFactory = ({ IDL }) => { const CanisterDetails = IDL.Record({ name: IDL.Text, canister_id: IDL.Principal, + block_index: IDL.Opt(IDL.Nat64), }); const ImportedToken = IDL.Record({ index_canister_id: IDL.Opt(IDL.Principal), diff --git a/frontend/src/lib/canisters/nns-dapp/nns-dapp.types.ts b/frontend/src/lib/canisters/nns-dapp/nns-dapp.types.ts index 5c30c23fbbc..673242828f4 100644 --- a/frontend/src/lib/canisters/nns-dapp/nns-dapp.types.ts +++ b/frontend/src/lib/canisters/nns-dapp/nns-dapp.types.ts @@ -12,6 +12,7 @@ export type AccountIdentifierString = string; export interface AttachCanisterRequest { name: string; canister_id: Principal; + block_index: [] | [bigint]; } export type AttachCanisterResponse = | { Ok: null } @@ -22,6 +23,7 @@ export type AttachCanisterResponse = export interface CanisterDetails { name: string; canister_id: CanisterId; + block_index: [] | [bigint]; } export type CanisterId = Principal; export type CreateSubAccountResponse = diff --git a/frontend/src/tests/lib/api/canisters.api.spec.ts b/frontend/src/tests/lib/api/canisters.api.spec.ts index fb10957ddf8..6e25bb5169b 100644 --- a/frontend/src/tests/lib/api/canisters.api.spec.ts +++ b/frontend/src/tests/lib/api/canisters.api.spec.ts @@ -228,7 +228,8 @@ describe("canisters-api", () => { }); it("should make a transfer, notify and attach the canister", async () => { - mockLedgerCanister.transfer.mockResolvedValue(10n); + const blockIndex = 10n; + mockLedgerCanister.transfer.mockResolvedValue(blockIndex); mockCMCCanister.notifyCreateCanister.mockResolvedValue( mockCanisterDetails.id ); @@ -243,12 +244,14 @@ describe("canisters-api", () => { expect(mockNNSDappCanister.attachCanister).toBeCalledWith({ name: "", canisterId: mockCanisterDetails.id, + blockIndex, }); expect(response).toEqual(mockCanisterDetails.id); }); it("should attach the canister if name max length", async () => { - mockLedgerCanister.transfer.mockResolvedValue(10n); + const blockIndex = 10n; + mockLedgerCanister.transfer.mockResolvedValue(blockIndex); mockCMCCanister.notifyCreateCanister.mockResolvedValue( mockCanisterDetails.id ); @@ -263,6 +266,7 @@ describe("canisters-api", () => { expect(mockNNSDappCanister.attachCanister).toBeCalledWith({ name: longName, canisterId: mockCanisterDetails.id, + blockIndex, }); expect(response).toEqual(mockCanisterDetails.id); }); @@ -283,7 +287,8 @@ describe("canisters-api", () => { }); it("handles creating from subaccounts", async () => { - mockLedgerCanister.transfer.mockResolvedValue(10n); + const blockIndex = 10n; + mockLedgerCanister.transfer.mockResolvedValue(blockIndex); mockCMCCanister.notifyCreateCanister.mockResolvedValue( mockCanisterDetails.id ); @@ -315,6 +320,7 @@ describe("canisters-api", () => { expect(mockNNSDappCanister.attachCanister).toBeCalledWith({ name: "", canisterId: mockCanisterDetails.id, + blockIndex, }); expect(response).toEqual(mockCanisterDetails.id); }); diff --git a/frontend/src/tests/lib/canisters/nns-dapp.canister.spec.ts b/frontend/src/tests/lib/canisters/nns-dapp.canister.spec.ts index 09de4916b83..1f6d4e5bebe 100644 --- a/frontend/src/tests/lib/canisters/nns-dapp.canister.spec.ts +++ b/frontend/src/tests/lib/canisters/nns-dapp.canister.spec.ts @@ -242,12 +242,20 @@ describe("NNSDapp", () => { service.attach_canister.mockResolvedValue({ Ok: null }); const nnsDapp = await createNnsDapp(service); + expect(service.attach_canister).toBeCalledTimes(0); + await nnsDapp.attachCanister({ name: "test", canisterId: mockCanister.canister_id, + blockIndex: 123n, }); - expect(service.attach_canister).toBeCalled(); + expect(service.attach_canister).toBeCalledWith({ + name: "test", + canister_id: mockCanister.canister_id, + block_index: [123n], + }); + expect(service.attach_canister).toBeCalledTimes(1); }); it("should throw CanisterAlreadyAttachedError", async () => { @@ -261,6 +269,7 @@ describe("NNSDapp", () => { nnsDapp.attachCanister({ name: "test", canisterId: mockCanister.canister_id, + blockIndex: 123n, }); expect(call).rejects.toThrowError(CanisterAlreadyAttachedError); @@ -277,6 +286,7 @@ describe("NNSDapp", () => { nnsDapp.attachCanister({ name: "test", canisterId: mockCanister.canister_id, + blockIndex: 123n, }); expect(call).rejects.toThrowError(CanisterNameAlreadyTakenError); @@ -293,6 +303,7 @@ describe("NNSDapp", () => { nnsDapp.attachCanister({ name: "test", canisterId: mockCanister.canister_id, + blockIndex: 123n, }); expect(call).rejects.toThrowError(CanisterNameTooLongError); @@ -309,6 +320,7 @@ describe("NNSDapp", () => { nnsDapp.attachCanister({ name: "test", canisterId: mockCanister.canister_id, + blockIndex: 123n, }); expect(call).rejects.toThrowError(CanisterLimitExceededError); diff --git a/frontend/src/tests/lib/pages/CanisterDetail.spec.ts b/frontend/src/tests/lib/pages/CanisterDetail.spec.ts index c9a51aa3f15..ed7b28cc5ac 100644 --- a/frontend/src/tests/lib/pages/CanisterDetail.spec.ts +++ b/frontend/src/tests/lib/pages/CanisterDetail.spec.ts @@ -1,6 +1,7 @@ import * as canisterApi from "$lib/api/canisters.api"; import * as icpIndexApi from "$lib/api/icp-index.api"; import { UserNotTheControllerError } from "$lib/canisters/ic-management/ic-management.errors"; +import type { CanisterDetails } from "$lib/canisters/nns-dapp/nns-dapp.types"; import CanisterDetail from "$lib/pages/CanisterDetail.svelte"; import { authStore } from "$lib/stores/auth.store"; import { canistersStore } from "$lib/stores/canisters.store"; @@ -57,6 +58,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: "", + block_index: [], }, ]); }); @@ -96,6 +98,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: canisterName, + block_index: [], }, ]); }); @@ -122,6 +125,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: "canister name", + block_index: [], }, ]); }); @@ -148,6 +152,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: "", + block_index: [], }, ]); const { queryByTestId } = render(CanisterDetail, props); @@ -165,6 +170,7 @@ describe("CanisterDetail", () => { { canister_id: Principal.fromText(canisterIdText), name: "", + block_index: [], }, ]); const po = await renderComponent(); @@ -177,6 +183,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: canisterName, + block_index: [], }, ]); const po = await renderComponent(); @@ -187,9 +194,10 @@ describe("CanisterDetail", () => { describe("rename button", () => { const newName = "new name"; const oldName = "old name"; - const canisterOldName = { + const canisterOldName: CanisterDetails = { canister_id: canisterId, name: oldName, + block_index: [], }; const canisterNewName = { ...canisterOldName, @@ -245,6 +253,7 @@ describe("CanisterDetail", () => { { canister_id: canisterId, name: "", + block_index: [], }, ]); }); diff --git a/frontend/src/tests/mocks/canisters.mock.ts b/frontend/src/tests/mocks/canisters.mock.ts index d9f97313808..7ddd8e00765 100644 --- a/frontend/src/tests/mocks/canisters.mock.ts +++ b/frontend/src/tests/mocks/canisters.mock.ts @@ -14,14 +14,16 @@ import { writable, type Subscriber } from "svelte/store"; import { mockIdentity } from "./auth.store.mock"; export const mockCanisterId = Principal.fromText("ryjl3-tyaaa-aaaaa-aaaba-cai"); -export const mockCanister = { +export const mockCanister: CanisterInfo = { name: "", canister_id: mockCanisterId, + block_index: [123n], }; export const mockCanisters: CanisterInfo[] = [ { name: "test1", canister_id: Principal.fromText("rrkah-fqaaa-aaaaa-aaaaq-cai"), + block_index: [123n], }, mockCanister, ];