feat: round ingress expiry (#788)

* feat: rounds expiry down to nearest minute

* adjusts expiry down for replica drift

* switching to default nonce strategy with note to fix

dfx.json

@@ -3,6 +3,10 @@
     "docs": {
       "type": "assets",
       "source": ["docs/generated"]
+    },
+    "counter": {
+      "type": "motoko",
+      "main": "e2e/node/canisters/counter.mo"
     }
   }
 }

docs/generated/changelog.html

@@ -13,6 +13,10 @@ <h1>Agent-JS Changelog</h1>
       <h2>Version x.x.x</h2>
       <ul>
         <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
+          nearest second. This matches existing behaviour, but adds the rounding
+        </li>
         <li>
           chore: replaces use of localhost with 127.0.0.1 for better node 18 support. Also swaps
           Jest for vitest, runs mitm against mainnet, and updates some packages

e2e/node/basic/counter.test.ts

@@ -1,3 +1,5 @@
+import { ActorSubclass } from '@dfinity/agent';
+import type { _SERVICE } from '../canisters/declarations/counter/index';
 import counterCanister, { noncelessCanister, createActor } from '../canisters/counter';
 import { it, expect, describe, vi } from 'vitest';
 
@@ -30,14 +32,20 @@ describe('counter', () => {
 
     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: counter } = await noncelessCanister();
+    const { actor } = await counterCanister();
+    const counter = actor as ActorSubclass<_SERVICE>;
 
+    await counter.write(BigInt(0));
     expect(Number(await counter.read())).toEqual(0);
-    await counter.inc();
-    expect(Number(await counter.read())).toEqual(1);
-    await counter.inc();
-    expect(Number(await counter.read())).toEqual(2);
+    let expected = 1;
+    for (let i = 0; i < 5; i++) {
+      await counter.inc();
+      expect(Number(await counter.read())).toEqual(expected);
+      expected += 1;
+    }
   }, 40000);
 });
 describe('retrytimes', () => {

e2e/node/canisters/counter.ts

@@ -11,6 +11,17 @@ let cache: {
   actor: any;
 } | null = null;
 
+const idl: IDL.InterfaceFactory = ({ IDL }) => {
+  return IDL.Service({
+    inc: IDL.Func([], [], []),
+    inc_read: IDL.Func([], [IDL.Nat], []),
+    read: IDL.Func([], [IDL.Nat], ['query']),
+    write: IDL.Func([IDL.Nat], [], []),
+    greet: IDL.Func([IDL.Text], [IDL.Text], []),
+    queryGreet: IDL.Func([IDL.Text], [IDL.Text], ['query']),
+  });
+};
+
 /**
  * Create a counter Actor + canisterId
  */
@@ -24,15 +35,6 @@ export default async function (): Promise<{
 
     const canisterId = await Actor.createCanister({ agent: await agent });
     await Actor.install({ module }, { canisterId, agent: await agent });
-    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']),
-      });
-    };
 
     cache = {
       canisterId,
@@ -59,20 +61,11 @@ export async function noncelessCanister(): Promise<{
 
   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']),
-    });
-  };
-
+  const actor = Actor.createActor(idl, { canisterId, agent: await disableNonceAgent }) as any;
   return {
     canisterId,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    actor: Actor.createActor(idl, { canisterId, agent: await disableNonceAgent }) as any,
+    actor,
   };
 }
 
@@ -91,14 +84,5 @@ export const createActor = async (options?: HttpAgentOptions) => {
 
   const canisterId = await Actor.createCanister({ agent });
   await Actor.install({ module }, { canisterId, agent });
-  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 Actor.createActor(idl, { canisterId, agent }) as any;
 };

e2e/node/canisters/declarations/counter/index.ts

@@ -1,4 +1,4 @@
-import { Actor, HttpAgent } from '@dfinity/agent';
+import { Actor, ActorMethod, HttpAgent } from '@dfinity/agent';
 
 // Imports and re-exports candid interface
 import { idlFactory } from './counter.did.js';
@@ -35,3 +35,12 @@ export const createActor = (canisterId, options = {}) => {
     ...options.actorOptions,
   });
 };
+
+export interface _SERVICE {
+  greet: ActorMethod<[string], string>;
+  inc: ActorMethod<[], undefined>;
+  inc_read: ActorMethod<[], bigint>;
+  queryGreet: ActorMethod<[string], string>;
+  read: ActorMethod<[], bigint>;
+  write: ActorMethod<[bigint], undefined>;
+}

e2e/node/tsconfig.json

@@ -4,7 +4,7 @@
     "incremental": true,
     "target": "ESNext",
     "module": "ESNext",
-    "lib": ["ES2023"],
+    "lib": ["ESNext"],
     "declaration": true,
     "sourceMap": true,
     "tsBuildInfoFile": "./build_info.json",

e2e/node/utils/agent.ts

@@ -1,6 +1,5 @@
 import { HttpAgent, HttpAgentOptions } from '@dfinity/agent';
 import { Ed25519KeyIdentity } from '@dfinity/identity';
-import fetch from 'isomorphic-fetch';
 
 export const identity = Ed25519KeyIdentity.generate();
 export const principal = identity.getPrincipal();
@@ -13,8 +12,6 @@ if (Number.isNaN(port)) {
 export const makeAgent = async (options?: HttpAgentOptions) => {
   const agent = new HttpAgent({
     host: `http://127.0.0.1:${process.env.REPLICA_PORT ?? 4943}`,
-    fetch: global.fetch ?? fetch,
-    verifyQuerySignatures: false,
     ...options,
   });
   try {

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

@@ -613,28 +613,7 @@ describe('retry failures', () => {
   });
 });
 jest.useFakeTimers({ legacyFakeTimers: true });
-test('should change nothing if time is within 30 seconds of replica', async () => {
-  const systemTime = new Date('August 19, 1975 23:15:30');
-  // jest.setSystemTime(systemTime);
-  const mockFetch = jest.fn();
-
-  const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });
-
-  await agent.syncTime();
 
-  agent
-    .call(Principal.managementCanister(), {
-      methodName: 'test',
-      arg: new Uint8Array().buffer,
-    })
-    // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
-    .catch(function (_) {});
-
-  const requestBody = cbor.decode(mockFetch.mock.calls[0][1].body);
-  expect((requestBody as unknown as any).content.ingress_expiry).toMatchInlineSnapshot(
-    `1240000000000`,
-  );
-});
 test('should adjust the Expiry if the clock is more than 30 seconds behind', async () => {
   const mockFetch = jest.fn();
 
@@ -669,11 +648,8 @@ test('should adjust the Expiry if the clock is more than 30 seconds behind', asy
   // Expiry should be: ingress expiry + replica time
   const expiryInMs = requestBody.content.ingress_expiry / NANOSECONDS_PER_MILLISECONDS;
 
-  const delay = expiryInMs + REPLICA_PERMITTED_DRIFT_MILLISECONDS - Number(replicaTime);
+  expect(requestBody.content.ingress_expiry).toMatchInlineSnapshot(`1260000000000`);
 
-  expect(requestBody.content.ingress_expiry).toMatchInlineSnapshot(`1271000000000`);
-
-  expect(delay).toBe(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS);
   jest.resetModules();
 });
 
@@ -709,14 +685,8 @@ test('should adjust the Expiry if the clock is more than 30 seconds ahead', asyn
 
   const requestBody: any = cbor.decode(mockFetch.mock.calls[0][1].body);
 
-  // Expiry should be: replica time - ingress expiry
-  const expiryInMs = requestBody.content.ingress_expiry / NANOSECONDS_PER_MILLISECONDS;
-
-  const delay = Number(replicaTime) - (expiryInMs + REPLICA_PERMITTED_DRIFT_MILLISECONDS);
-
-  expect(requestBody.content.ingress_expiry).toMatchInlineSnapshot(`1209000000000`);
+  expect(requestBody.content.ingress_expiry).toMatchInlineSnapshot(`1200000000000`);
 
-  expect(delay).toBe(-1 * DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS);
   jest.resetModules();
 });
 

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

@@ -0,0 +1,15 @@
+import { Expiry } from './transforms';
+
+jest.useFakeTimers();
+test('it should round down to the nearest minute', () => {
+  // 2021-04-26T17:47:11.314Z - high precision
+  jest.setSystemTime(new Date(1619459231314));
+
+  const expiry = new Expiry(5 * 60 * 1000);
+  expect(expiry['_value']).toEqual(BigInt(1619459460000000000));
+
+  const expiry_as_date_string = new Date(
+    Number(expiry['_value'] / BigInt(1_000_000)),
+  ).toISOString();
+  expect(expiry_as_date_string).toBe('2021-04-26T17:51:00.000Z');
+});

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

@@ -8,20 +8,29 @@ import {
   makeNonce,
   Nonce,
 } from './types';
-import { toHex } from '../../utils/buffer';
 
 const NANOSECONDS_PER_MILLISECONDS = BigInt(1_000_000);
 
-const REPLICA_PERMITTED_DRIFT_MILLISECONDS = BigInt(60 * 1000);
+const REPLICA_PERMITTED_DRIFT_MILLISECONDS = 60 * 1000;
 
 export class Expiry {
   private readonly _value: bigint;
 
   constructor(deltaInMSec: number) {
     // Use bigint because it can overflow the maximum number allowed in a double float.
-    this._value =
-      (BigInt(Date.now()) + BigInt(deltaInMSec) - REPLICA_PERMITTED_DRIFT_MILLISECONDS) *
+    const raw_value =
+      BigInt(Math.floor(Date.now() + deltaInMSec - REPLICA_PERMITTED_DRIFT_MILLISECONDS)) *
       NANOSECONDS_PER_MILLISECONDS;
+
+    // round down to the nearest second
+    const ingress_as_seconds = raw_value / BigInt(1_000_000_000);
+
+    // round down to nearest minute
+    const ingress_as_minutes = ingress_as_seconds / BigInt(60);
+
+    const rounded_down_nanos = ingress_as_minutes * BigInt(60) * BigInt(1_000_000_000);
+
+    this._value = rounded_down_nanos;
   }
 
   public toCBOR(): cbor.CborValue {