From a091cb8a70e8e65f4af2777110cc055c8da6da7e Mon Sep 17 00:00:00 2001
From: Kyle Peacock <kylpeacock@gmail.com>
Date: Thu, 7 Apr 2022 10:27:42 -0700
Subject: [PATCH] feat: generate nonce for HttpAgent calls (#554)

* feat: includes nonce generation in httpagent by default
---
 demos/ledgerhq/src/main.js                 |  2 -
 docs/generated/changelog.html              | 13 +++++++
 e2e/node/basic/counter.test.ts             | 24 +++++++++++-
 e2e/node/canisters/counter.ts              | 43 +++++++++++++++++++++-
 e2e/node/utils/agent.ts                    |  7 ++--
 packages/agent/src/actor.test.ts           |  2 +-
 packages/agent/src/agent/http/http.test.ts | 13 +++++--
 packages/agent/src/agent/http/index.ts     | 21 ++++++++++-
 8 files changed, 110 insertions(+), 15 deletions(-)

diff --git a/demos/ledgerhq/src/main.js b/demos/ledgerhq/src/main.js
index 4657b34ac..1078d460b 100644
--- a/demos/ledgerhq/src/main.js
+++ b/demos/ledgerhq/src/main.js
@@ -82,8 +82,6 @@ document.getElementById('sendBtn').addEventListener('click', async () => {
 
   // Need to run a replica locally which has ledger canister running on it
   const agent = new HttpAgent({ host, identity });
-  // Ledger Hardware Wallet requires that the request must contain a nonce
-  agent.addTransform(makeNonceTransform());
 
   const resp = await agent.call(canisterId, {
     methodName: document.getElementById('methodName').value,
diff --git a/docs/generated/changelog.html b/docs/generated/changelog.html
index 5d75f50f0..628d9da97 100644
--- a/docs/generated/changelog.html
+++ b/docs/generated/changelog.html
@@ -28,6 +28,19 @@ <h2>Version 0.10.5</h2>
           versions to 0 for major version updates
         </li>
         <li>Removes jest-expect-message, which was making test error messages less useful</li>
+        <li>
+          <p>
+            HttpAgent now generates a nonce to ensure that calls are unique by default. If you want
+            to opt out or provide your own nonce logic, you can now pass an option of
+            <code>disableNonce: true</code>during the agent initialization.
+          </p>
+          <p>
+            If you are currently using
+            <code>agent.addTransform(makeNonceTransform())</code>
+            , please note that you should remove that logic, or add the <code>disableNonce</code>
+            option to your agent when upgrading.
+          </p>
+        </li>
       </ul>
       <h2>Version 0.10.3</h2>
       <ul>
diff --git a/e2e/node/basic/counter.test.ts b/e2e/node/basic/counter.test.ts
index f04417916..342dc1630 100644
--- a/e2e/node/basic/counter.test.ts
+++ b/e2e/node/basic/counter.test.ts
@@ -1,9 +1,9 @@
 /**
  * @jest-environment node
  */
-import counterCanister from '../canisters/counter';
+import counterCanister, { noncelessCanister } from '../canisters/counter';
 
-jest.setTimeout(30000);
+jest.setTimeout(40000);
 describe('counter', () => {
   it('should greet', async () => {
     const { actor: counter } = await counterCanister();
@@ -13,6 +13,26 @@ describe('counter', () => {
       console.error(error);
     }
   });
+  it('should submit distinct requests with nonce by default', async () => {
+    const { actor: counter } = await counterCanister();
+    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);
+
+    // Sets of unique results should be the same length
+    expect(set1.size).toBe(values.length);
+    expect(set2.size).toEqual(values2.length);
+  });
+  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);
+  });
   it('should increment', async () => {
     const { actor: counter } = await counterCanister();
     try {
diff --git a/e2e/node/canisters/counter.ts b/e2e/node/canisters/counter.ts
index f4846235a..d0d090f08 100644
--- a/e2e/node/canisters/counter.ts
+++ b/e2e/node/canisters/counter.ts
@@ -1,9 +1,9 @@
-import { Actor } from '@dfinity/agent';
+import { Actor, HttpAgent } from '@dfinity/agent';
 import { IDL } from '@dfinity/candid';
 import { Principal } from '@dfinity/principal';
 import { readFileSync } from 'fs';
 import path from 'path';
-import agent from '../utils/agent';
+import agent, { port, identity } from '../utils/agent';
 
 let cache: {
   canisterId: Principal;
@@ -27,6 +27,7 @@ export default async function (): Promise<{
     const idl: IDL.InterfaceFactory = ({ IDL }) => {
       return IDL.Service({
         inc: IDL.Func([], [], []),
+        inc_read: IDL.Func([], [IDL.Nat], []),
         read: IDL.Func([], [IDL.Nat], ['query']),
         greet: IDL.Func([IDL.Text], [IDL.Text], []),
         queryGreet: IDL.Func([IDL.Text], [IDL.Text], ['query']),
@@ -42,3 +43,41 @@ 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 Promise.resolve(
+    new HttpAgent({
+      host: 'http://127.0.0.1:' + port,
+      identity,
+      disableNonce: true,
+    }),
+  ).then(async agent => {
+    await agent.fetchRootKey();
+    return agent;
+  });
+
+  const canisterId = await Actor.createCanister({ agent: disableNonceAgent });
+  await Actor.install({ module }, { canisterId, agent: disableNonceAgent });
+  const idl: IDL.InterfaceFactory = ({ IDL }) => {
+    return IDL.Service({
+      inc: IDL.Func([], [], []),
+      inc_read: IDL.Func([], [IDL.Nat], []),
+      read: IDL.Func([], [IDL.Nat], ['query']),
+      greet: IDL.Func([IDL.Text], [IDL.Text], []),
+      queryGreet: IDL.Func([IDL.Text], [IDL.Text], ['query']),
+    });
+  };
+
+  return {
+    canisterId,
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    actor: Actor.createActor(idl, { canisterId, agent: await disableNonceAgent }) as any,
+  };
+}
diff --git a/e2e/node/utils/agent.ts b/e2e/node/utils/agent.ts
index 7a6ba2a50..23e7e97dc 100644
--- a/e2e/node/utils/agent.ts
+++ b/e2e/node/utils/agent.ts
@@ -1,10 +1,10 @@
-import { HttpAgent, makeNonceTransform } from '@dfinity/agent';
+import { HttpAgent } from '@dfinity/agent';
 import { Ed25519KeyIdentity } from '@dfinity/identity';
 
-const identity = Ed25519KeyIdentity.generate();
+export const identity = Ed25519KeyIdentity.generate();
 export const principal = identity.getPrincipal();
 
-const port = parseInt(process.env['REPLICA_PORT'] || '', 10);
+export const port = parseInt(process.env['REPLICA_PORT'] || '', 10);
 if (Number.isNaN(port)) {
   throw new Error('The environment variable REPLICA_PORT is not a number.');
 }
@@ -12,7 +12,6 @@ if (Number.isNaN(port)) {
 const agent = Promise.resolve(new HttpAgent({ host: 'http://127.0.0.1:' + port, identity })).then(
   async agent => {
     await agent.fetchRootKey();
-    agent.addTransform(makeNonceTransform());
     return agent;
   },
 );
diff --git a/packages/agent/src/actor.test.ts b/packages/agent/src/actor.test.ts
index 98604ab57..49d136f1c 100644
--- a/packages/agent/src/actor.test.ts
+++ b/packages/agent/src/actor.test.ts
@@ -98,7 +98,7 @@ describe('makeActor', () => {
 
     let nonceCount = 0;
 
-    const httpAgent = new HttpAgent({ fetch: mockFetch });
+    const httpAgent = new HttpAgent({ fetch: mockFetch, disableNonce: true });
     httpAgent.addTransform(makeNonceTransform(() => nonces[nonceCount++]));
 
     const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent });
diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts
index a32d5de86..e68ddfd43 100644
--- a/packages/agent/src/agent/http/http.test.ts
+++ b/packages/agent/src/agent/http/http.test.ts
@@ -49,7 +49,6 @@ test('call', async () => {
   const principal = Principal.anonymous();
 
   const httpAgent = new HttpAgent({ fetch: mockFetch, host: 'http://localhost' });
-  httpAgent.addTransform(makeNonceTransform(() => nonce));
 
   const methodName = 'greet';
   const arg = new Uint8Array([]);
@@ -116,7 +115,11 @@ test('queries with the same content should have the same signature', async () =>
 
   const principal = await Principal.anonymous();
 
-  const httpAgent = new HttpAgent({ fetch: mockFetch, host: 'http://localhost' });
+  const httpAgent = new HttpAgent({
+    fetch: mockFetch,
+    host: 'http://localhost',
+    disableNonce: true,
+  });
   httpAgent.addTransform(makeNonceTransform(() => nonce));
 
   const methodName = 'greet';
@@ -188,7 +191,11 @@ test('use anonymous principal if unspecified', async () => {
   const nonce = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce;
   const principal = Principal.anonymous();
 
-  const httpAgent = new HttpAgent({ fetch: mockFetch, host: 'http://localhost' });
+  const httpAgent = new HttpAgent({
+    fetch: mockFetch,
+    host: 'http://localhost',
+    disableNonce: true,
+  });
   httpAgent.addTransform(makeNonceTransform(() => nonce));
 
   const methodName = 'greet';
diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts
index bf69a48af..f59e96b53 100644
--- a/packages/agent/src/agent/http/index.ts
+++ b/packages/agent/src/agent/http/index.ts
@@ -13,13 +13,14 @@ import {
   ReadStateResponse,
   SubmitResponse,
 } from '../api';
-import { Expiry } from './transforms';
+import { Expiry, makeNonceTransform } from './transforms';
 import {
   CallRequest,
   Endpoint,
   HttpAgentRequest,
   HttpAgentRequestTransformFn,
   HttpAgentSubmitRequest,
+  makeNonce,
   QueryRequest,
   ReadRequestType,
   SubmitRequestType,
@@ -83,6 +84,19 @@ export interface HttpAgentOptions {
     name: string;
     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.
+   *
+   * 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 });
+   * agent.addTransform(makeNonceTransform(makeNonce);
+   * @default false
+   */
+  disableNonce?: boolean;
 }
 
 function getDefaultFetch(): typeof fetch {
@@ -178,6 +192,11 @@ export class HttpAgent implements Agent {
       this._credentials = `${name}${password ? ':' + password : ''}`;
     }
     this._identity = Promise.resolve(options.identity || new AnonymousIdentity());
+
+    // Add a nonce transform to ensure calls are unique
+    if (!options.disableNonce) {
+      this.addTransform(makeNonceTransform(makeNonce));
+    }
   }
 
   public addTransform(fn: HttpAgentRequestTransformFn, priority = fn.priority || 0): void {