feat: target_canister to be handled only for method install_chunked_c…

…ode (#957) * feat: target_canister to be handled only for method install_chunked_code * chore: formatting and changelog entry --------- Co-authored-by: Kai Peacock <[email protected]>
dfinity · Dec 12, 2024 · 7ed119f · 7ed119f
1 parent 2e8d15a
commit 7ed119f
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 38 deletions.
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Added
 
+- fix: `target_canister` is used only for `install_chunked_code` of management canister, complying with internet computer specification
 - chore: pins nanoid dev dependency version to override warning
 - feat: Add support for effective target canister ID in management canister calls.
 

diff --git a/packages/agent/src/actor.ts b/packages/agent/src/actor.ts
@@ -657,20 +657,20 @@ export type ManagementCanisterRecord = _SERVICE;
  */
 export function getManagementCanister(config: CallConfig): ActorSubclass<ManagementCanisterRecord> {
   function transform(
-    _methodName: string,
+    methodName: string,
     args: Record<string, unknown> & { canister_id: string; target_canister?: unknown }[],
   ) {
     if (config.effectiveCanisterId) {
       return { effectiveCanisterId: Principal.from(config.effectiveCanisterId) };
     }
     const first = args[0];
     let effectiveCanisterId = Principal.fromHex('');
+    if (first && typeof first === 'object' && first.target_canister && methodName === "install_chunked_code") {
+      effectiveCanisterId = Principal.from(first.target_canister);
+    }
     if (first && typeof first === 'object' && first.canister_id) {
       effectiveCanisterId = Principal.from(first.canister_id as unknown);
     }
-    if (first && typeof first === 'object' && first.target_canister) {
-      effectiveCanisterId = Principal.from(first.target_canister);
-    }
     return { effectiveCanisterId };
   }
 

diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts
@@ -18,6 +18,8 @@ import {
   AnonymousIdentity,
   fromHex,
   getManagementCanister,
+  type ManagementCanisterRecord,
+  type ActorSubclass,
   SignIdentity,
   toHex,
 } from '../..';
@@ -938,7 +940,7 @@ test('it should handle calls against the ic-management canister that succeed', a
   expect(status).toMatchSnapshot();
 });
 
-test('it should use target_canister as effective canister id for calls against the ic-management canister', async () => {
+describe('transform', () => {
   const identity = new AnonymousIdentity();
 
   // Response generated by calling a locally deployed replica of the management canister, cloned using fetchCloner
@@ -965,49 +967,86 @@ test('it should use target_canister as effective canister id for calls against t
     });
   });
 
-  // Mock time so certificates can be accurately decoded
-  jest.useFakeTimers();
-  jest.setSystemTime(mockResponse.now);
+  let management: ActorSubclass<ManagementCanisterRecord>;
+  let spy: jest.SpyInstance;
+
+  beforeEach(async () => {
+    // Mock time so certificates can be accurately decoded
+    jest.useFakeTimers();
+    jest.setSystemTime(mockResponse.now);
+
+    // Pass in rootKey from replica (used because test was written using local replica)
+    const agent = await HttpAgent.createSync({
+      identity,
+      fetch: mockFetch,
+      host: 'http://localhost:4943',
+      rootKey: fromHex(
+        '308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c050302010361008be882f1985cccb53fd551571a42818014835ed8f8a27767669b67dd4a836eb0d62b327e3368a80615b0e4f472c73f7917c036dc9317dcb64b319a1efa43dd7c656225c061de359db6fdf7033ac1bff24c944c145e46ebdce2093680b6209a13',
+      ),
+    });
 
-  // Pass in rootKey from replica (used because test was written using local replica)
-  const agent = await HttpAgent.createSync({
-    identity,
-    fetch: mockFetch,
-    host: 'http://localhost:4943',
-    rootKey: fromHex(
-      '308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c050302010361008be882f1985cccb53fd551571a42818014835ed8f8a27767669b67dd4a836eb0d62b327e3368a80615b0e4f472c73f7917c036dc9317dcb64b319a1efa43dd7c656225c061de359db6fdf7033ac1bff24c944c145e46ebdce2093680b6209a13',
-    ),
+    // Use management canister call
+    management = getManagementCanister({ agent });
+
+    // Important - override nonce when making request to ensure reproducible result
+    (Actor.agentOf(management) as HttpAgent).addTransform('update', async args => {
+      args.body.nonce = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce;
+      return args;
+    });
+
+    spy = jest.spyOn(Actor.agentOf(management) as HttpAgent, 'call');
   });
 
-  // Use management canister call
-  const management = getManagementCanister({ agent });
+  test('it should use target_canister as effective canister id for calls against the ic-management canister', async () => {
+    const target_canister = Principal.from('bkyz2-fmaaa-aaaaa-qaaaq-cai');
+
+    await management.install_chunked_code({
+      arg: new Uint8Array([1, 2, 3]),
+      wasm_module_hash: new Uint8Array([4, 5, 6]),
+      mode: { install: null },
+      chunk_hashes_list: [],
+      target_canister,
+      store_canister: [],
+      sender_canister_version: [],
+    });
 
-  // Important - override nonce when making request to ensure reproducible result
-  (Actor.agentOf(management) as HttpAgent).addTransform('update', async args => {
-    args.body.nonce = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce;
-    return args;
+    expect(spy).toHaveBeenCalledWith(
+      Principal.fromHex(''),
+      expect.objectContaining({
+        effectiveCanisterId: target_canister,
+      }),
+    );
   });
 
-  const spy = jest.spyOn(Actor.agentOf(management) as HttpAgent, 'call');
+  test('it should use canister_id as effective canister id for calls against the ic-management canister if target_canister is provided but install_chunked_code is not', async () => {
+    const target_canister = Principal.from('ryjl3-tyaaa-aaaaa-aaaba-cai');
+    const canister_id = Principal.from('bkyz2-fmaaa-aaaaa-qaaaq-cai');
 
-  const target_canister = Principal.from('bkyz2-fmaaa-aaaaa-qaaaq-cai');
+    await management.stop_canister({
+      canister_id,
+      target_canister,
+    } as unknown as { canister_id: Principal; target_canister_id: Principal });
 
-  await management.install_chunked_code({
-    arg: new Uint8Array([1, 2, 3]),
-    wasm_module_hash: new Uint8Array([4, 5, 6]),
-    mode: { install: null },
-    chunk_hashes_list: [],
-    target_canister,
-    store_canister: [],
-    sender_canister_version: [],
+    expect(spy).toHaveBeenCalledWith(
+      Principal.fromHex(''),
+      expect.objectContaining({
+        effectiveCanisterId: canister_id,
+      }),
+    );
   });
 
-  expect(spy).toHaveBeenCalledWith(
-    Principal.fromHex(''),
-    expect.objectContaining({
-      effectiveCanisterId: target_canister,
-    }),
-  );
+  test('it should use canister_id as effective canister id for calls against the ic-management canister', async () => {
+    const canister_id = Principal.from('bkyz2-fmaaa-aaaaa-qaaaq-cai');
+
+    await management.stop_canister({ canister_id });
+
+    expect(spy).toHaveBeenCalledWith(
+      Principal.fromHex(''),
+      expect.objectContaining({
+        effectiveCanisterId: canister_id,
+      }),
+    );
+  });
 });
 
 /**