Skip to content

Commit

Permalink
feat: refactor nonce specification in HttpAgent
Browse files Browse the repository at this point in the history
  • Loading branch information
krpeacock committed Oct 16, 2024
1 parent 00ad493 commit 835ff94
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 23 deletions.
6 changes: 6 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 18 additions & 5 deletions packages/agent/src/actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand All @@ -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": [],
Expand All @@ -318,6 +329,7 @@ describe('makeActor', () => {
"_value": 1200000000000n,
},
"method_name": "greet",
"nonce": Uint8Array [],
"request_type": "query",
"sender": {
"__principal__": "2vxsx-fae",
Expand All @@ -329,6 +341,7 @@ describe('makeActor', () => {
`);
expect(replyUpdateWithHttpDetails.result).toEqual(canisterDecodedReturnValue);

assert(replyUpdateWithHttpDetails.httpDetails);
replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array();

expect(replyUpdateWithHttpDetails.httpDetails).toMatchSnapshot();
Expand Down
12 changes: 11 additions & 1 deletion packages/agent/src/agent/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions packages/agent/src/agent/http/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
23 changes: 10 additions & 13 deletions packages/agent/src/agent/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
SubmitResponse,
v3ResponseBody,
} from '../api';
import { Expiry, httpHeadersTransform, makeNonceTransform } from './transforms';
import { Expiry, httpHeadersTransform } from './transforms';
import {
CallRequest,
Endpoint,
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -410,6 +405,7 @@ export class HttpAgent implements Agent {
arg: ArrayBuffer;
effectiveCanisterId?: Principal | string;
callSync?: boolean;
nonce?: Nonce;
},
identity?: Identity | Promise<Identity>,
): Promise<SubmitResponse> {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down
90 changes: 90 additions & 0 deletions packages/agent/src/agent/http/nonce.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
11 changes: 10 additions & 1 deletion packages/agent/src/agent/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 835ff94

Please sign in to comment.