feat: refactor to remove nonce from queries (#792)

* replaces disableNonce feature with useQueryNonces

* changelog

docs/generated/changelog.html

@@ -12,6 +12,11 @@ <h1>Agent-JS Changelog</h1>
     <section>
       <h2>Version x.x.x</h2>
       <ul>
+        <li>
+          feat!: replaces disableNonce feature with useQueryNonces. Going forward, updates will use
+          nonces, but queries and readstate calls will not. Queries and readsatate calls will use
+          nonces if `useQueryNonces` is set to true
+        </li>
         <li>feat: adds subnet metrics decoding to canisterStatus for `/subnet` path</li>
         <li>
           feat!: sets expiry to 1 minute less than the configured expiry, and then down to the

e2e/node/basic/counter.test.ts

@@ -1,6 +1,4 @@
-import { ActorSubclass } from '@dfinity/agent';
-import type { _SERVICE } from '../canisters/declarations/counter/index';
-import counterCanister, { noncelessCanister, createActor } from '../canisters/counter';
+import counterCanister, { createActor } from '../canisters/counter';
 import { it, expect, describe, vi } from 'vitest';
 
 describe('counter', () => {
@@ -23,22 +21,10 @@ describe('counter', () => {
     expect(set1.size).toBe(values.length);
     expect(set2.size).toEqual(values2.length);
   }, 40000);
-  it('should submit duplicate requests if nonce is disabled', async () => {
-    const { actor: counter } = await noncelessCanister();
-    const values = await Promise.all(new Array(4).fill(undefined).map(() => counter.inc_read()));
-    const set1 = new Set(values);
-    const values2 = await Promise.all(new Array(4).fill(undefined).map(() => counter.inc_read()));
-    const set2 = new Set(values2);
-
-    expect(set1.size < values.length || set2.size < values2.length).toBe(true);
-  }, 40000);
-  // FIX: Run same test with nonceless canister once
-  // https://dfinity.atlassian.net/browse/BOUN-937 is fixed
   it('should increment', async () => {
-    const { actor } = await counterCanister();
-    const counter = actor as ActorSubclass<_SERVICE>;
+    const { actor: counter } = await counterCanister();
 
-    await counter.write(BigInt(0));
+    await counter.write(0);
     expect(Number(await counter.read())).toEqual(0);
     let expected = 1;
     for (let i = 0; i < 5; i++) {

e2e/node/canisters/counter.ts

@@ -45,29 +45,6 @@ export default async function (): Promise<{
 
   return cache;
 }
-/**
- * With no cache and nonce disabled
- */
-export async function noncelessCanister(): Promise<{
-  canisterId: Principal;
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  actor: any;
-}> {
-  const module = readFileSync(path.join(__dirname, 'counter.wasm'));
-  const disableNonceAgent = await makeAgent({
-    identity,
-    disableNonce: true,
-  });
-
-  const canisterId = await Actor.createCanister({ agent: disableNonceAgent });
-  await Actor.install({ module }, { canisterId, agent: disableNonceAgent });
-  const actor = Actor.createActor(idl, { canisterId, agent: await disableNonceAgent }) as any;
-  return {
-    canisterId,
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    actor,
-  };
-}
 
 export const createActor = async (options?: HttpAgentOptions) => {
   const module = readFileSync(path.join(__dirname, 'counter.wasm'));

packages/agent/src/actor.test.ts

@@ -1,7 +1,7 @@
 import { IDL } from '@dfinity/candid';
 import { Principal } from '@dfinity/principal';
 import { HttpAgent, Nonce, SubmitResponse } from './agent';
-import { Expiry, makeNonceTransform } from './agent/http/transforms';
+import { Expiry } from './agent/http/transforms';
 import { CallRequest, SubmitRequestType, UnSigned } from './agent/http/types';
 import * as cbor from './cbor';
 import { requestIdOf } from './request_id';
@@ -133,10 +133,7 @@ describe('makeActor', () => {
 
     const expectedCallRequestId = await requestIdOf(expectedCallRequest.content);
 
-    let nonceCount = 0;
-
-    const httpAgent = new HttpAgent({ fetch: mockFetch, disableNonce: true });
-    httpAgent.addTransform(makeNonceTransform(() => nonces[nonceCount++]));
+    const httpAgent = new HttpAgent({ fetch: mockFetch });
 
     const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent });
     const reply = await actor.greet(argValue);

packages/agent/src/agent/http/http.test.ts

@@ -1,7 +1,7 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { HttpAgent, Nonce } from '../index';
 import * as cbor from '../../cbor';
-import { Expiry, httpHeadersTransform, makeNonceTransform } from './transforms';
+import { Expiry, httpHeadersTransform } from './transforms';
 import {
   CallRequest,
   Envelope,
@@ -24,8 +24,6 @@ window.fetch = global.fetch;
 
 const HTTP_AGENT_HOST = 'http://127.0.0.1:4943';
 
-const DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS = 5 * 60 * 1000;
-const REPLICA_PERMITTED_DRIFT_MILLISECONDS = 60 * 1000;
 const NANOSECONDS_PER_MILLISECONDS = 1_000_000;
 
 function createIdentity(seed: number): Ed25519KeyIdentity {
@@ -139,9 +137,7 @@ test('queries with the same content should have the same signature', async () =>
   const httpAgent = new HttpAgent({
     fetch: mockFetch,
     host: 'http://127.0.0.1',
-    disableNonce: true,
   });
-  httpAgent.addTransform(makeNonceTransform(() => nonce));
 
   const methodName = 'greet';
   const arg = new Uint8Array([]);
@@ -198,13 +194,12 @@ test('readState should not call transformers if request is passed', async () =>
   const httpAgent = new HttpAgent({
     fetch: mockFetch,
     host: 'http://127.0.0.1',
-    disableNonce: true,
+    useQueryNonces: true,
   });
-  httpAgent.addTransform(makeNonceTransform(() => nonce));
   const transformMock: HttpAgentRequestTransformFn = jest
     .fn()
     .mockImplementation(d => Promise.resolve(d));
-  httpAgent.addTransform(transformMock);
+  httpAgent.addTransform('query', transformMock);
 
   const methodName = 'greet';
   const arg = new Uint8Array([]);
@@ -288,9 +283,7 @@ test('use anonymous principal if unspecified', async () => {
   const httpAgent = new HttpAgent({
     fetch: mockFetch,
     host: 'http://127.0.0.1',
-    disableNonce: true,
   });
-  httpAgent.addTransform(makeNonceTransform(() => nonce));
 
   const methodName = 'greet';
   const arg = new Uint8Array([]);

packages/agent/src/agent/http/index.ts

@@ -103,18 +103,15 @@ export interface HttpAgentOptions {
     password?: string;
   };
   /**
-   * Prevents the agent from providing a unique {@link Nonce} with each call.
-   * Enabling may cause rate limiting of identical requests
-   * at the boundary nodes.
+   * Adds a unique {@link Nonce} with each query.
+   * Enabling will prevent queries from being answered with a cached response.
    *
-   * To add your own nonce generation logic, you can use the following:
    * @example
-   * import {makeNonceTransform, makeNonce} from '@dfinity/agent';
-   * const agent = new HttpAgent({ disableNonce: true });
+   * const agent = new HttpAgent({ useQueryNonces: true });
    * agent.addTransform(makeNonceTransform(makeNonce);
    * @default false
    */
-  disableNonce?: boolean;
+  useQueryNonces?: boolean;
   /**
    * Number of times to retry requests before throwing an error
    * @default 3
@@ -168,7 +165,6 @@ function getDefaultFetch(): typeof fetch {
 // allowing extensions.
 export class HttpAgent implements Agent {
   public rootKey = fromHex(IC_ROOT_KEY);
-  private readonly _pipeline: HttpAgentRequestTransformFn[] = [];
   private _identity: Promise<Identity> | null;
   private readonly _fetch: typeof fetch;
   private readonly _fetchOptions?: Record<string, unknown>;
@@ -180,14 +176,16 @@ export class HttpAgent implements Agent {
   private readonly _retryTimes; // Retry requests N times before erroring by default
   public readonly _isAgent = true;
 
+  #queryPipeline: HttpAgentRequestTransformFn[] = [];
+  #updatePipeline: HttpAgentRequestTransformFn[] = [];
+
   #subnetKeys: Map<string, SubnetStatus> = new Map();
 
   constructor(options: HttpAgentOptions = {}) {
     if (options.source) {
       if (!(options.source instanceof HttpAgent)) {
         throw new Error("An Agent's source can only be another HttpAgent");
       }
-      this._pipeline = [...options.source._pipeline];
       this._identity = options.source._identity;
       this._fetch = options.source._fetch;
       this._host = options.source._host;
@@ -253,8 +251,9 @@ export class HttpAgent implements Agent {
     this._identity = Promise.resolve(options.identity || new AnonymousIdentity());
 
     // Add a nonce transform to ensure calls are unique
-    if (!options.disableNonce) {
-      this.addTransform(makeNonceTransform(makeNonce));
+    this.addTransform('update', makeNonceTransform(makeNonce));
+    if (options.useQueryNonces) {
+      this.addTransform('query', makeNonceTransform(makeNonce));
     }
   }
 
@@ -263,10 +262,28 @@ export class HttpAgent implements Agent {
     return hostname === '127.0.0.1' || hostname.endsWith('127.0.0.1');
   }
 
-  public addTransform(fn: HttpAgentRequestTransformFn, priority = fn.priority || 0): void {
-    // Keep the pipeline sorted at all time, by priority.
-    const i = this._pipeline.findIndex(x => (x.priority || 0) < priority);
-    this._pipeline.splice(i >= 0 ? i : this._pipeline.length, 0, Object.assign(fn, { priority }));
+  public addTransform(
+    type: 'update' | 'query',
+    fn: HttpAgentRequestTransformFn,
+    priority = fn.priority || 0,
+  ): void {
+    if (type === 'update') {
+      // Keep the pipeline sorted at all time, by priority.
+      const i = this.#updatePipeline.findIndex(x => (x.priority || 0) < priority);
+      this.#updatePipeline.splice(
+        i >= 0 ? i : this.#updatePipeline.length,
+        0,
+        Object.assign(fn, { priority }),
+      );
+    } else if (type === 'query') {
+      // Keep the pipeline sorted at all time, by priority.
+      const i = this.#queryPipeline.findIndex(x => (x.priority || 0) < priority);
+      this.#queryPipeline.splice(
+        i >= 0 ? i : this.#queryPipeline.length,
+        0,
+        Object.assign(fn, { priority }),
+      );
+    }
   }
 
   public async getPrincipal(): Promise<Principal> {
@@ -609,9 +626,14 @@ export class HttpAgent implements Agent {
 
   protected _transform(request: HttpAgentRequest): Promise<HttpAgentRequest> {
     let p = Promise.resolve(request);
-
-    for (const fn of this._pipeline) {
-      p = p.then(r => fn(r).then(r2 => r2 || r));
+    if (request.endpoint === Endpoint.Call) {
+      for (const fn of this.#updatePipeline) {
+        p = p.then(r => fn(r).then(r2 => r2 || r));
+      }
+    } else {
+      for (const fn of this.#queryPipeline) {
+        p = p.then(r => fn(r).then(r2 => r2 || r));
+      }
     }
 
     return p;

packages/agent/src/agent/http/transforms.ts

@@ -50,7 +50,6 @@ export class Expiry {
  */
 export function makeNonceTransform(nonceFn: () => Nonce = makeNonce): HttpAgentRequestTransformFn {
   return async (request: HttpAgentRequest) => {
-    const nonce = nonceFn();
     // Nonce needs to be inserted into the header for all requests, to enable logs to be correlated with requests.
     const headers = request.request.headers;
     // TODO: uncomment this when the http proxy supports it.