fix: account for clock drift in certificate freshness check (#1081)

Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com>

Author: ilbertt

CHANGELOG.md

@@ -4,11 +4,15 @@
 
 - fix: do not subtract the replica permitted clock drift when calculating the ingress expiry.
 - fix: pick the expiry rounding strategy based on the delta, without adding the clock drift to the delta.
-- feat: adds a `clockDriftMs` optional parameter to `Expiry.fromDeltaInMilliseconds` to add to the current time, typically used to specify the clock drift between the client's clock and the IC network clock.
-- fix: add declaration maps and typescript source code to published packages
+- feat: adds a `clockDriftMs` optional parameter to `Expiry.fromDeltaInMilliseconds` to add to the current time, typically used to specify the clock drift between the IC network clock and the client's clock.
+- fix: add declaration maps and typescript source code to published packages.
 - feat: enables type inference for the arguments and return types of `FuncClass`.
 - feat: enables type inference for the fields of `ServiceClass`.
 - fix: perform the canister range checks unconditionally for delegations when constructing a `Certificate` instance.
+- fix: account for clock drift when verifying the certificate freshness, and syncs time with the IC network if the certificate fails the freshness check and the agent's time is not already synced.
+- feat: adds the `agent` optional field to the `CreateCertificateOptions` interface, which is used to sync time with the IC network if the certificate fails the freshness check, if provided.
+- feat: adds the `getTimeDiffMsecs` method to the `HttpAgent` class, which returns the time difference in milliseconds between the IC network clock and the client's clock.
+- feat: adds the `hasSyncedTime` method to the `HttpAgent` class, which returns `true` if the time has been synced at least once with the IC network, `false` otherwise.
 
 ## [3.1.0] - 2025-07-24
 

e2e/node/basic/syncTime.test.ts

@@ -94,6 +94,7 @@ describe('syncTime', () => {
         },
         'V3 call body',
       );
+      expect(agent.hasSyncedTime()).toBe(false);
     });
 
     it('should sync time when the local time does not match the subnet time', async () => {
@@ -179,6 +180,7 @@ describe('syncTime', () => {
         },
         'V3 read state body three',
       );
+      expect(agent.hasSyncedTime()).toBe(true);
     });
   });
 
@@ -197,7 +199,7 @@ describe('syncTime', () => {
         res.status(200).send(readStateResponse);
       });
 
-      await HttpAgent.create({
+      const agent = await HttpAgent.create({
         host: mockReplica.address,
         rootKey: keyPair.publicKeyDer,
         shouldSyncTime: true,
@@ -225,6 +227,7 @@ describe('syncTime', () => {
         },
         'V3 read state body three',
       );
+      expect(agent.hasSyncedTime()).toBe(true);
     });
 
     it('should not sync time by default', async () => {
@@ -235,14 +238,15 @@ describe('syncTime', () => {
         res.status(200).send(readStateResponse);
       });
 
-      await HttpAgent.create({
+      const agent = await HttpAgent.create({
         host: mockReplica.address,
         rootKey: keyPair.publicKeyDer,
         shouldSyncTime: false,
         identity: anonIdentity,
       });
 
       expect(mockReplica.getV2ReadStateSpy(ICP_LEDGER)).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(false);
     });
 
     it('should not sync time when explicitly disabled', async () => {
@@ -253,14 +257,15 @@ describe('syncTime', () => {
         res.status(200).send(readStateResponse);
       });
 
-      await HttpAgent.create({
+      const agent = await HttpAgent.create({
         host: mockReplica.address,
         rootKey: keyPair.publicKeyDer,
         shouldSyncTime: false,
         identity: anonIdentity,
       });
 
       expect(mockReplica.getV2ReadStateSpy(ICP_LEDGER)).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(false);
     });
   });
 
@@ -340,6 +345,7 @@ describe('syncTime', () => {
         },
         'V3 read state body three',
       );
+      expect(agent.hasSyncedTime()).toBe(true);
     });
 
     it('should not sync time by default', async () => {
@@ -389,6 +395,7 @@ describe('syncTime', () => {
       );
 
       expect(mockReplica.getV2ReadStateSpy(canisterId.toString())).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(false);
     });
 
     it('should not sync time when explicitly disabled', async () => {
@@ -440,6 +447,7 @@ describe('syncTime', () => {
       );
 
       expect(mockReplica.getV2ReadStateSpy(canisterId.toString())).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(false);
     });
   });
 });

packages/agent/src/actor.ts

@@ -427,6 +427,7 @@ function _createActorMethod(
           rootKey: agent.rootKey,
           canisterId: Principal.from(canisterId),
           blsVerify,
+          agent,
         });
         const path = [utf8ToBytes('request_status'), requestId];
         const status = new TextDecoder().decode(

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

@@ -85,6 +85,8 @@ export enum RequestStatusResponseStatus {
 const MINUTE_TO_MSECS = 60 * 1_000;
 const MSECS_TO_NANOSECONDS = 1_000_000;
 
+const DEFAULT_TIME_DIFF_MSECS = 0;
+
 // Root public key for the IC, encoded as hex
 export const IC_ROOT_KEY =
   '308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c05030201036100814' +
@@ -286,7 +288,8 @@ export class HttpAgent implements Agent {
   #rootKeyPromise: Promise<Uint8Array> | null = null;
   readonly #shouldFetchRootKey: boolean = false;
 
-  #timeDiffMsecs = 0;
+  #timeDiffMsecs = DEFAULT_TIME_DIFF_MSECS;
+  #hasSyncedTime = false;
   #syncTimePromise: Promise<void> | null = null;
   readonly #shouldSyncTime: boolean = false;
 
@@ -313,7 +316,7 @@ export class HttpAgent implements Agent {
   #updatePipeline: HttpAgentRequestTransformFn[] = [];
 
   #subnetKeys: ExpirableMap<string, SubnetStatus> = new ExpirableMap({
-    expirationTime: 5 * 60 * 1000, // 5 minutes
+    expirationTime: 5 * MINUTE_TO_MSECS,
   });
   #verifyQuerySignatures = true;
 
@@ -949,7 +952,13 @@ export class HttpAgent implements Agent {
     // Attempt to make the query i=retryTimes times
     // Make query and fetch subnet keys in parallel
     try {
-      const [queryResult, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]);
+      const [queryResult, subnetStatus] = await Promise.all([
+        makeQuery(),
+        getSubnetStatus().catch(err => {
+          this.log.warn('Failed to fetch subnet keys. Error:', err);
+          return undefined;
+        }),
+      ]);
       const { requestDetails, query } = queryResult;
 
       const queryWithDetails = {
@@ -968,10 +977,8 @@ export class HttpAgent implements Agent {
       } catch {
         // In case the node signatures have changed, refresh the subnet keys and try again
         this.log.warn('Query response verification failed. Retrying with fresh subnet keys.');
-        this.#subnetKeys.delete(canisterId.toString());
-        await this.fetchSubnetKeys(ecid.toString());
-
-        const updatedSubnetStatus = this.#subnetKeys.get(canisterId.toString());
+        this.#subnetKeys.delete(ecid.toString());
+        const updatedSubnetStatus = await getSubnetStatus();
         if (!updatedSubnetStatus) {
           throw TrustError.fromCode(new MissingSignatureErrorCode());
         }
@@ -1208,8 +1215,8 @@ export class HttpAgent implements Agent {
       }
       const date = decodeTime(timeLookup.value);
       this.log.print('Time from response:', date);
-      this.log.print('Time from response in milliseconds:', Number(date));
-      return Number(date);
+      this.log.print('Time from response in milliseconds:', date.getTime());
+      return date.getTime();
     } else {
       this.log.warn('No certificate found in response');
     }
@@ -1241,12 +1248,14 @@ export class HttpAgent implements Agent {
             fetch: this.#fetch,
             retryTimes: 0,
             rootKey: this.rootKey ?? undefined,
+            shouldSyncTime: false,
           });
 
           const replicaTimes = await Promise.all(
             Array(3)
               .fill(null)
               .map(async () => {
+                // TODO: disable certificate freshness check for this request
                 const status = await canisterStatusRequest({
                   canisterId,
                   agent: anonymousAgent,
@@ -1264,8 +1273,9 @@ export class HttpAgent implements Agent {
             return typeof current === 'number' && current > max ? current : max;
           }, 0);
 
-          if (maxReplicaTime > BigInt(0)) {
-            this.#timeDiffMsecs = Number(maxReplicaTime) - Number(callTime);
+          if (maxReplicaTime > 0) {
+            this.#timeDiffMsecs = maxReplicaTime - callTime;
+            this.#hasSyncedTime = true;
             this.log.notify({
               message: `Syncing time: offset of ${this.#timeDiffMsecs}`,
               level: 'info',
@@ -1341,7 +1351,7 @@ export class HttpAgent implements Agent {
   }
 
   async #syncTimeGuard(canisterIdOverride?: Principal): Promise<void> {
-    if (this.#shouldSyncTime && this.#timeDiffMsecs === 0) {
+    if (this.#shouldSyncTime && !this.hasSyncedTime()) {
       await this.syncTime(canisterIdOverride);
     }
   }
@@ -1386,6 +1396,23 @@ export class HttpAgent implements Agent {
 
     return p;
   }
+
+  /**
+   * Returns the time difference in milliseconds between the IC network clock and the client's clock,
+   * after the clock has been synced.
+   *
+   * If the time has not been synced, returns `0`.
+   */
+  public getTimeDiffMsecs(): number {
+    return this.#timeDiffMsecs;
+  }
+
+  /**
+   * Returns `true` if the time has been synced at least once with the IC network, `false` otherwise.
+   */
+  public hasSyncedTime(): boolean {
+    return this.#hasSyncedTime;
+  }
 }
 
 /**

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

@@ -38,7 +38,7 @@ export class Expiry {
    * If the delta is less than 90 seconds, the expiry is rounded down to the nearest second.
    * Otherwise, the expiry is rounded down to the nearest minute.
    * @param deltaInMs The milliseconds to add to the current time.
-   * @param clockDriftMs The milliseconds to add to the current time, typically the clock drift between the client and the IC network clock. Defaults to `0` if not provided.
+   * @param clockDriftMs The milliseconds to add to the current time, typically the clock drift between IC network clock and the client's clock. Defaults to `0` if not provided.
    * @returns {Expiry} The constructed Expiry object.
    */
   public static fromDeltaInMilliseconds(deltaInMs: number, clockDriftMs: number = 0): Expiry {