fix(agent): sync time if ingress expiry is invalid in read_state

CHANGELOG.md

@@ -24,6 +24,7 @@ See the [v5 upgrading guide](https://js.icp.build/core/latest/upgrading/v5/) for
 - fix(agent): check if canister is in ranges for certificates without delegation
 - fix(agent): verify all query signatures instead of only the first one
 - fix(agent): sync time if ingress expiry is invalid in queries
+- fix(agent): sync time if ingress expiry is invalid in read_state
 - fix(agent): sync time before retrying if query signature is outdated
 - fix(agent,identity/secp256k1): remove `console.*` statements
 - refactor(agent): only declare IC URLs once in the `HttpAgent` class

docs/src/content/docs/upgrading/v5.md

@@ -217,7 +217,7 @@ The `@dfinity/assets` package has been deprecated. Its functionality is now part
 
 ## Fixes
 
-This release includes minor security improvements to how query responses are validated, strengthening certificate verification and query signature checks.
+This release includes minor security improvements to how `query` and `read_state` responses are validated, strengthening certificate verification and query/read_state signature checks.
 
 ---
 

e2e/node/basic/syncTime.test.ts

@@ -46,6 +46,7 @@ describe('syncTime', () => {
 
   const rootSubnetKeyPair = randomKeyPair();
   const keyPair = randomKeyPair();
+  const subnetId = Principal.selfAuthenticating(keyPair.publicKeyDer);
   const nodeIdentity = randomIdentity();
   const identity = randomIdentity();
   const anonIdentity = new AnonymousIdentity();
@@ -279,6 +280,98 @@ describe('syncTime', () => {
       expect(mockReplica.getV3ReadStateSpy(canisterId.toString())).toHaveBeenCalledTimes(4);
       expect(agent.hasSyncedTime()).toBe(true);
     });
+
+    it('should sync time when the local time does not match the subnet time (read_state for canister)', async () => {
+      const agent = await HttpAgent.create({
+        host: mockReplica.address,
+        rootKey: rootSubnetKeyPair.publicKeyDer,
+        identity,
+      });
+
+      mockReplica.setV3ReadStateSpyImplOnce(canisterId.toString(), (_req, res) => {
+        res.status(400).send(new TextEncoder().encode(INVALID_EXPIRY_ERROR));
+      });
+
+      await mockSyncTimeResponse({
+        mockReplica,
+        rootSubnetKeyPair,
+        keyPair,
+        canisterId,
+      });
+
+      // Mock sync time for subnet to check that it is not called as expected
+      await mockSyncSubnetTimeResponse({
+        mockReplica,
+        rootSubnetKeyPair,
+        keyPair,
+      });
+
+      const { responseBody: readStateResponse } = await prepareV3ReadStateResponse({
+        nodeIdentity,
+        canisterRanges: [[canisterId.toUint8Array(), canisterId.toUint8Array()]],
+        rootSubnetKeyPair,
+        keyPair,
+        date,
+      });
+      mockReplica.setV3ReadStateSpyImplOnce(canisterId.toString(), (_req, res) => {
+        res.status(200).send(readStateResponse);
+      });
+
+      const response = await agent.readState(canisterId, {
+        paths: [[new TextEncoder().encode('time')]],
+      });
+
+      expect(response.certificate).toBeDefined();
+      expect(mockReplica.getV3ReadStateSpy(canisterId.toString())).toHaveBeenCalledTimes(5);
+      expect(mockReplica.getV3ReadSubnetStateSpy(subnetId.toString())).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(true);
+    });
+
+    it('should sync time when the local time does not match the subnet time (read_state for subnet)', async () => {
+      const agent = await HttpAgent.create({
+        host: mockReplica.address,
+        rootKey: rootSubnetKeyPair.publicKeyDer,
+        identity,
+      });
+
+      mockReplica.setV3ReadSubnetStateSpyImplOnce(subnetId.toString(), (_req, res) => {
+        res.status(400).send(new TextEncoder().encode(INVALID_EXPIRY_ERROR));
+      });
+
+      await mockSyncSubnetTimeResponse({
+        mockReplica,
+        rootSubnetKeyPair,
+        keyPair,
+      });
+
+      // Mock sync time for canister to check that it is not called as expected
+      await mockSyncTimeResponse({
+        mockReplica,
+        rootSubnetKeyPair,
+        keyPair,
+        canisterId,
+      });
+
+      const { responseBody: readStateResponse } = await prepareV3ReadStateResponse({
+        nodeIdentity,
+        canisterRanges: [], // not needed for subnet time
+        rootSubnetKeyPair,
+        keyPair,
+        date,
+      });
+      mockReplica.setV3ReadSubnetStateSpyImplOnce(subnetId.toString(), (_req, res) => {
+        res.status(200).send(readStateResponse);
+      });
+
+      const response = await agent.readSubnetState(subnetId, {
+        paths: [[new TextEncoder().encode('time')]],
+      });
+
+      expect(response.certificate).toBeDefined();
+      expect(mockReplica.getV3ReadSubnetStateSpy(subnetId.toString())).toHaveBeenCalledTimes(5);
+      expect(mockReplica.getV3ReadStateSpy(canisterId.toString())).toHaveBeenCalledTimes(0);
+      expect(agent.hasSyncedTime()).toBe(true);
+    });
   });
 
   describe('on async creation', () => {
@@ -544,6 +637,7 @@ describe('syncTimeWithSubnet', () => {
 
   const rootSubnetKeyPair = randomKeyPair();
   const keyPair = randomKeyPair();
+  const subnetId = Principal.selfAuthenticating(keyPair.publicKeyDer);
   const identity = randomIdentity();
 
   let mockReplica: MockReplica;
@@ -575,7 +669,6 @@ describe('syncTimeWithSubnet', () => {
 
     expect(agent.hasSyncedTime()).toBe(false);
 
-    const subnetId = Principal.selfAuthenticating(keyPair.publicKeyDer);
     await agent.syncTimeWithSubnet(subnetId);
 
     expect(mockReplica.getV3ReadSubnetStateSpy(subnetId.toString())).toHaveBeenCalledTimes(3);

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

@@ -27,6 +27,7 @@ import {
   MalformedLookupFoundValueErrorCode,
   CertificateOutdatedErrorCode,
   CertificateNotAuthorizedErrorCode,
+  UNREACHABLE_ERROR,
 } from '../../errors.ts';
 import { AnonymousIdentity, type Identity } from '../../auth.ts';
 import * as cbor from '../../cbor.ts';
@@ -1142,7 +1143,7 @@ export class HttpAgent implements Agent {
 
     const url = new URL(`/api/v3/canister/${canister.toString()}/read_state`, this.host);
 
-    return await this.#readStateInner(url, transformedRequest, requestId);
+    return await this.#readStateInner(url, { canisterId: canister }, transformedRequest, requestId);
   }
 
   /**
@@ -1156,18 +1157,20 @@ export class HttpAgent implements Agent {
     options: ReadStateOptions,
   ): Promise<ReadStateResponse> {
     await this.#rootKeyGuard();
+    const subnet = Principal.from(subnetId);
 
-    const url = new URL(`/api/v3/subnet/${subnetId.toString()}/read_state`, this.host);
+    const url = new URL(`/api/v3/subnet/${subnet.toString()}/read_state`, this.host);
     const transformedRequest: ReadStateRequest = await this.createReadStateRequest(
       options,
       this.#identity ?? undefined,
     );
 
-    return await this.#readStateInner(url, transformedRequest);
+    return await this.#readStateInner(url, { subnetId: subnet }, transformedRequest);
   }
 
   async #readStateInner(
     url: URL,
+    principal: { canisterId: Principal } | { subnetId: Principal },
     transformedRequest: ReadStateRequest,
     requestId?: RequestId,
   ): Promise<ReadStateResponse> {
@@ -1190,6 +1193,18 @@ export class HttpAgent implements Agent {
     } catch (error) {
       let readStateError: AgentError;
       if (error instanceof AgentError) {
+        if (error.hasCode(IngressExpiryInvalidErrorCode) && !this.#hasSyncedTime) {
+          // if there is an ingress expiry error and the time has not been synced yet,
+          // sync time with the network and try again
+          if ('canisterId' in principal) {
+            await this.syncTime(principal.canisterId);
+          } else if ('subnetId' in principal) {
+            await this.syncTimeWithSubnet(principal.subnetId);
+          } else {
+            throw UNREACHABLE_ERROR;
+          }
+          return await this.#readStateInner(url, principal, transformedRequest, requestId);
+        }
         // override the error code to include the request details
         error.code.requestContext = {
           requestId: requestId ?? requestIdOf(transformedRequest),