Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: refactor nonce specification in HttpAgent #944

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
Loading