From 835ff944f789a00dbbb2bf4911b97f3cd2d7be2c Mon Sep 17 00:00:00 2001 From: Kai Peacock Date: Wed, 16 Oct 2024 15:41:28 -0700 Subject: [PATCH] feat: refactor nonce specification in HttpAgent --- docs/CHANGELOG.md | 6 ++ packages/agent/src/actor.test.ts | 23 ++++-- packages/agent/src/agent/api.ts | 12 ++- packages/agent/src/agent/http/http.test.ts | 6 +- packages/agent/src/agent/http/index.ts | 23 +++--- packages/agent/src/agent/http/nonce.test.ts | 90 +++++++++++++++++++++ packages/agent/src/agent/http/types.ts | 11 ++- 7 files changed, 148 insertions(+), 23 deletions(-) create mode 100644 packages/agent/src/agent/http/nonce.test.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 93c6d18f..f2f6acae 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,12 @@ ## Changed +- feat: refactor nonce from `transform` stragegy to more explicit option in `call` and `query` methods. + - QueryFields now includes optional `nonce` field + - default is still not to include a nonce unless `useQueryNonces` is set to true + - CallOptions now includes optional `nonce` field + - default is still to automatically generate a nonce unless one is provided +- feat: `makeNonce` now returns a `Uint8Array` with the attribute `__nonce__` set to `undefined`, making it usable in JavaScript instead of just type checking. `Object.hasOwn(nonce, '__nonce__')` will return `true` for nonces generated by `makeNonce` - fix: recalculates body to use a fresh `Expiry` when polling for `read_state` requests. This prevents the request from exceeding the `maximum_ingress_expiry` when the replica is slow to respond. ## [2.1.2] - 2024-09-30 diff --git a/packages/agent/src/actor.test.ts b/packages/agent/src/actor.test.ts index d15483d6..9013a52f 100644 --- a/packages/agent/src/actor.test.ts +++ b/packages/agent/src/actor.test.ts @@ -6,7 +6,8 @@ import { CallRequest, SubmitRequestType, UnSigned } from './agent/http/types'; import * as cbor from './cbor'; import { requestIdOf } from './request_id'; import * as pollingImport from './polling'; -import { Actor, ActorConfig } from './actor'; +import { ActorConfig } from './actor'; +import assert from 'assert'; const importActor = async (mockUpdatePolling?: () => void) => { jest.dontMock('./polling'); @@ -279,10 +280,16 @@ describe('makeActor', () => { }); const canisterId = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c'); const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent }); - const actorWithHttpDetails = Actor.createActorWithHttpDetails(actorInterface, { - canisterId, - agent: httpAgent, - }); + const actorWithHttpDetails = Actor.createActorWithExtendedDetails( + actorInterface, + { + canisterId, + agent: httpAgent, + }, + { + httpDetails: true, + }, + ); const reply = await actor.greet('test'); const replyUpdate = await actor.greet_update('test'); @@ -292,6 +299,10 @@ describe('makeActor', () => { expect(reply).toEqual(canisterDecodedReturnValue); expect(replyUpdate).toEqual(canisterDecodedReturnValue); expect(replyWithHttpDetails.result).toEqual(canisterDecodedReturnValue); + + assert(replyWithHttpDetails.httpDetails); + replyWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); + expect(replyWithHttpDetails.httpDetails).toMatchInlineSnapshot(` { "headers": [], @@ -318,6 +329,7 @@ describe('makeActor', () => { "_value": 1200000000000n, }, "method_name": "greet", + "nonce": Uint8Array [], "request_type": "query", "sender": { "__principal__": "2vxsx-fae", @@ -329,6 +341,7 @@ describe('makeActor', () => { `); expect(replyUpdateWithHttpDetails.result).toEqual(canisterDecodedReturnValue); + assert(replyUpdateWithHttpDetails.httpDetails); replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); expect(replyUpdateWithHttpDetails.httpDetails).toMatchSnapshot(); diff --git a/packages/agent/src/agent/api.ts b/packages/agent/src/agent/api.ts index b521116b..39888509 100644 --- a/packages/agent/src/agent/api.ts +++ b/packages/agent/src/agent/api.ts @@ -2,7 +2,7 @@ import { Principal } from '@dfinity/principal'; import { RequestId } from '../request_id'; import { JsonObject } from '@dfinity/candid'; import { Identity } from '../auth'; -import { CallRequest, HttpHeaderField, QueryRequest } from './http/types'; +import { CallRequest, HttpHeaderField, Nonce, QueryRequest } from './http/types'; /** * Codes used by the replica for rejecting a message. @@ -94,6 +94,11 @@ export interface QueryFields { * Overrides canister id for path to fetch. This is used for management canister calls. */ effectiveCanisterId?: Principal; + + /** + * Optional nonce to use for the query. Can be used to avoid caching. By default, the agent will not use a nonce for queries. + */ + nonce?: Nonce; } /** @@ -115,6 +120,11 @@ export interface CallOptions { * @see https://internetcomputer.org/docs/current/references/ic-interface-spec/#http-effective-canister-id */ effectiveCanisterId: Principal | string; + + /** + * Optional nonce to use for the update. Can be used to avoid caching. By default, the agent will generate a nonce for updates automatically. + */ + nonce?: Nonce; } export interface ReadStateResponse { diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 386b6bda..0b88cb54 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -13,7 +13,7 @@ import { Principal } from '@dfinity/principal'; import { requestIdOf } from '../../request_id'; import { JSDOM } from 'jsdom'; -import { Actor, AnonymousIdentity, SignIdentity, toHex } from '../..'; +import { AnonymousIdentity, SignIdentity, toHex } from '../..'; import { Ed25519KeyIdentity } from '@dfinity/identity'; import { AgentError } from '../../errors'; import { AgentHTTPResponseError } from './errors'; @@ -159,8 +159,8 @@ test('queries with the same content should have the same signature', async () => const response1 = await httpAgent.readState(canisterIdent, { paths }); const response2 = await httpAgent.readState(canisterIdent, { paths }); - const response3 = await httpAgent.query(canisterIdent, { arg, methodName }); - const response4 = await httpAgent.query(canisterIdent, { methodName, arg }); + const response3 = await httpAgent.query(canisterIdent, { arg, methodName, nonce }); + const response4 = await httpAgent.query(canisterIdent, { methodName, arg, nonce }); const { calls } = mockFetch.mock; expect(calls.length).toBe(4); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index f3aa840e..bbcb3ca8 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -15,7 +15,7 @@ import { SubmitResponse, v3ResponseBody, } from '../api'; -import { Expiry, httpHeadersTransform, makeNonceTransform } from './transforms'; +import { Expiry, httpHeadersTransform } from './transforms'; import { CallRequest, Endpoint, @@ -310,11 +310,6 @@ export class HttpAgent implements Agent { } this.#identity = Promise.resolve(options.identity || new AnonymousIdentity()); - // Add a nonce transform to ensure calls are unique - this.addTransform('update', makeNonceTransform(makeNonce)); - if (options.useQueryNonces) { - this.addTransform('query', makeNonceTransform(makeNonce)); - } if (options.logToConsole) { this.log.subscribe(log => { if (log.level === 'error') { @@ -410,6 +405,7 @@ export class HttpAgent implements Agent { arg: ArrayBuffer; effectiveCanisterId?: Principal | string; callSync?: boolean; + nonce?: Nonce; }, identity?: Identity | Promise, ): Promise { @@ -458,16 +454,10 @@ export class HttpAgent implements Agent { body: submit, })) as HttpAgentSubmitRequest; - const nonce: Nonce | undefined = transformedRequest.body.nonce - ? toNonce(transformedRequest.body.nonce) - : undefined; + const nonce: Nonce = options.nonce ?? makeNonce(); submit.nonce = nonce; - function toNonce(buf: ArrayBuffer): Nonce { - return new Uint8Array(buf) as Nonce; - } - // Apply transform for identity. transformedRequest = await id.transformRequest(transformedRequest); @@ -790,6 +780,13 @@ export class HttpAgent implements Agent { body: request, }); + // Insert nonce if provided or generate a new one if useQueryNonces is enabled + let nonce: Nonce | undefined = fields.nonce; + if (this.config.useQueryNonces === true || nonce === undefined) { + nonce = makeNonce(); + } + request.nonce = nonce; + // Apply transform for identity. transformedRequest = (await id?.transformRequest(transformedRequest)) as HttpAgentRequest; diff --git a/packages/agent/src/agent/http/nonce.test.ts b/packages/agent/src/agent/http/nonce.test.ts new file mode 100644 index 00000000..e3272fa1 --- /dev/null +++ b/packages/agent/src/agent/http/nonce.test.ts @@ -0,0 +1,90 @@ +import { Principal } from '@dfinity/principal'; +import { HttpAgent, makeNonce, Nonce } from './index'; +import * as cbor from '../../cbor'; +import { IDL } from '@dfinity/candid'; +import { assert } from 'console'; +import { QueryResponseStatus } from '../api'; + +describe('Nonce Generation', () => { + it('should generate a uint8array', () => { + const nonce = makeNonce(); + + expect(nonce).toBeInstanceOf(Uint8Array); + expect(Object.hasOwn(nonce, '__nonce__')).toBe(true); + }); + + it('should provide a nonce when passed to an agent query', async () => { + const expectedReplyArg = IDL.encode([IDL.Text], ['Hello, World!']); + const nonce = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce; + const mockFetch = jest.fn(() => { + return Promise.resolve( + new Response( + cbor.encode({ + status: 'replied', + reply: { + arg: expectedReplyArg, + }, + }), + { + status: 200, + statusText: 'ok', + }, + ), + ); + }); + const agent = HttpAgent.createSync({ + fetch: mockFetch, + host: 'http://127.0.0.1', + verifyQuerySignatures: false, + }); + const canisterId = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c'); + + const response = await agent.query(canisterId, { + methodName: 'read_state', + arg: new ArrayBuffer(0), + nonce, + }); + assert(response.status === QueryResponseStatus.Replied); + expect(response.requestDetails?.nonce).toBe(nonce); + + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('should provide a nonce when passed to an agent call', async () => { + const expectedReplyArg = IDL.encode([IDL.Text], ['Hello, World!']); + const nonce = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce; + const mockFetch = jest.fn(() => { + return Promise.resolve( + new Response( + cbor.encode({ + status: 'replied', + reply: { + arg: expectedReplyArg, + }, + }), + { + status: 200, + statusText: 'ok', + }, + ), + ); + }); + const agent = HttpAgent.createSync({ + fetch: mockFetch, + host: 'http://127.0.0.1', + verifyQuerySignatures: false, + }); + const canisterId = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c'); + + const { response, requestDetails } = await agent.call(canisterId, { + methodName: 'read_state', + arg: new ArrayBuffer(0), + nonce, + }); + + expect(response.status).toBe(200); + expect(requestDetails?.nonce).toBe(nonce); + + expect(mockFetch).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/agent/src/agent/http/types.ts b/packages/agent/src/agent/http/types.ts index f245c5cc..505c7f30 100644 --- a/packages/agent/src/agent/http/types.ts +++ b/packages/agent/src/agent/http/types.ts @@ -119,5 +119,14 @@ export function makeNonce(): Nonce { view.setUint32(8, rand3); view.setUint32(12, rand4); - return buffer as Nonce; + const nonce = new Uint8Array(buffer); + // add __nonce__ to the buffer to make it a Nonce + Object.defineProperty(nonce, '__nonce__', { + value: undefined, + writable: false, + enumerable: false, + configurable: false, + }); + + return nonce as Nonce; }