From fa40abe5ac569a80e4e2f6e78421d8751ec30ed1 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 16 Jul 2024 09:02:16 -0700 Subject: [PATCH 1/4] wip --- packages/agent/src/agent/http/http.test.ts | 20 ++++++++++++++++ packages/agent/src/agent/http/index.ts | 28 +++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 09d9cb67..9b1b4beb 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -809,3 +809,23 @@ test('it should log errors to console if the option is set', async () => { const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: jest.fn(), logToConsole: true }); await agent.syncTime(); }); + + +test.only('it should allow for configuring a max age in minutes for the ingress expiry', async () => { + const mockFetch = jest.fn(); + + const agent = await HttpAgent.create({ + host: HTTP_AGENT_HOST, + fetch: mockFetch, + ingressExpiryInMinutes: 5, + }); + + await agent.syncTime(); + + await agent.call(Principal.managementCanister(), { + methodName: 'test', + arg: new Uint8Array().buffer, + }); + + const requestBody: any = cbor.decode(mockFetch.mock.calls[0][1].body); +}); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 8c105797..edd2dfb4 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -53,8 +53,7 @@ export enum RequestStatusResponseStatus { Done = 'done', } -// Default delta for ingress expiry is 5 minutes. -const DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS = 5 * 60 * 1000; +const MINUTE_TO_MSECS = 60 * 1000; // Root public key for the IC, encoded as hex export const IC_ROOT_KEY = @@ -107,6 +106,12 @@ export interface HttpAgentOptions { // time (will throw). identity?: Identity | Promise; + /** + * The maximum time a request can be delayed before being rejected. + * @default 5 minutes + */ + ingressExpiryInMinutes?: number; + credentials?: { name: string; password?: string; @@ -243,6 +248,7 @@ export class HttpAgent implements Agent { #rootKeyFetched = false; readonly #retryTimes; // Retry requests N times before erroring by default #backoffStrategy: BackoffStrategyFactory; + readonly #maxIngressExpiryInMinutes: number; // Public signature to help with type checking. public readonly _isAgent = true; @@ -304,6 +310,14 @@ export class HttpAgent implements Agent { } this.#identity = Promise.resolve(options.identity || new AnonymousIdentity()); + if (options.ingressExpiryInMinutes && options.ingressExpiryInMinutes > 5) { + throw new AgentError( + `The maximum ingress expiry time is 5 minutes. Provided ingress expiry time is ${options.ingressExpiryInMinutes} minutes.`, + ); + } + + this.#maxIngressExpiryInMinutes = options.ingressExpiryInMinutes || 5; + // Add a nonce transform to ensure calls are unique this.addTransform('update', makeNonceTransform(makeNonce)); if (options.useQueryNonces) { @@ -419,11 +433,13 @@ export class HttpAgent implements Agent { const sender: Principal = id.getPrincipal() || Principal.anonymous(); - let ingress_expiry = new Expiry(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS); + let ingress_expiry = new Expiry(this.#maxIngressExpiryInMinutes * MINUTE_TO_MSECS); // If the value is off by more than 30 seconds, reconcile system time with the network if (Math.abs(this.#timeDiffMsecs) > 1_000 * 30) { - ingress_expiry = new Expiry(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS + this.#timeDiffMsecs); + ingress_expiry = new Expiry( + this.#maxIngressExpiryInMinutes * MINUTE_TO_MSECS + this.#timeDiffMsecs, + ); } const submit: CallRequest = { @@ -713,7 +729,7 @@ export class HttpAgent implements Agent { method_name: fields.methodName, arg: fields.arg, sender, - ingress_expiry: new Expiry(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS), + ingress_expiry: new Expiry(this.#maxIngressExpiryInMinutes * MINUTE_TO_MSECS), }; const requestId = await requestIdOf(request); @@ -900,7 +916,7 @@ export class HttpAgent implements Agent { request_type: ReadRequestType.ReadState, paths: fields.paths, sender, - ingress_expiry: new Expiry(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS), + ingress_expiry: new Expiry(this.#maxIngressExpiryInMinutes * MINUTE_TO_MSECS), }, }); From bb52652fc2a1cf0dc9cf9456e1ca60f4de2f28e5 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Thu, 18 Jul 2024 10:59:44 -0700 Subject: [PATCH 2/4] feat: allow for setting HttpAgent ingress expiry using `ingressExpiryInMinutes` option --- docs/CHANGELOG.md | 4 ++ e2e/node/basic/mainnet.test.ts | 46 ++++++++++++++++++++++ packages/agent/src/agent/http/http.test.ts | 25 ++++-------- packages/agent/src/agent/http/index.ts | 5 +++ 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 48f2a12f..94c0f938 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- feat: allow for setting HttpAgent ingress expiry using `ingressExpiryInMinutes` option + ## [2.0.0] - 2024-07-16 ### Changed diff --git a/e2e/node/basic/mainnet.test.ts b/e2e/node/basic/mainnet.test.ts index 3692d233..fd844f6a 100644 --- a/e2e/node/basic/mainnet.test.ts +++ b/e2e/node/basic/mainnet.test.ts @@ -140,3 +140,49 @@ describe('call forwarding', () => { reply; // ArrayBuffer }, 15_000); }); + +// TODO: change Expiry logic rounding to make <= 1 minute expiry work +test.skip('it should succeed when setting an expiry in the near future', async () => { + ``; + const agent = await HttpAgent.create({ + host: 'https://icp-api.io', + ingressExpiryInMinutes: 1, + }); + + await agent.syncTime(); + + expect( + agent.call('tnnnb-2yaaa-aaaab-qaiiq-cai', { + methodName: 'inc_read', + arg: fromHex('4449444c0000'), + effectiveCanisterId: 'tnnnb-2yaaa-aaaab-qaiiq-cai', + }), + ).rejects.toThrowError(`Specified ingress_expiry not within expected range`); +}); + +test('it should succeed when setting an expiry in the future', async () => { + ``; + const agent = await HttpAgent.create({ + host: 'https://icp-api.io', + ingressExpiryInMinutes: 5, + }); + + await agent.syncTime(); + + expect( + agent.call('tnnnb-2yaaa-aaaab-qaiiq-cai', { + methodName: 'inc_read', + arg: fromHex('4449444c0000'), + effectiveCanisterId: 'tnnnb-2yaaa-aaaab-qaiiq-cai', + }), + ).resolves.toBeDefined(); +}); + +test('it should fail when setting an expiry in the far future', async () => { + expect( + HttpAgent.create({ + host: 'https://icp-api.io', + ingressExpiryInMinutes: 100, + }), + ).rejects.toThrowError(`The maximum ingress expiry time is 5 minutes`); +}); diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 9b1b4beb..04256f6e 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -811,21 +811,12 @@ test('it should log errors to console if the option is set', async () => { }); -test.only('it should allow for configuring a max age in minutes for the ingress expiry', async () => { - const mockFetch = jest.fn(); - - const agent = await HttpAgent.create({ - host: HTTP_AGENT_HOST, - fetch: mockFetch, - ingressExpiryInMinutes: 5, - }); - - await agent.syncTime(); - - await agent.call(Principal.managementCanister(), { - methodName: 'test', - arg: new Uint8Array().buffer, - }); - - const requestBody: any = cbor.decode(mockFetch.mock.calls[0][1].body); +test('it should fail when setting an expiry in the past', async () => { + expect(() => + HttpAgent.createSync({ + host: 'https://icp-api.io', + ingressExpiryInMinutes: -1, + fetch: jest.fn(), + }), + ).toThrow(`Ingress expiry time must be greater than 0`); }); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index edd2dfb4..366773ea 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -315,6 +315,11 @@ export class HttpAgent implements Agent { `The maximum ingress expiry time is 5 minutes. Provided ingress expiry time is ${options.ingressExpiryInMinutes} minutes.`, ); } + if (options.ingressExpiryInMinutes && options.ingressExpiryInMinutes <= 0) { + throw new AgentError( + `Ingress expiry time must be greater than 0. Provided ingress expiry time is ${options.ingressExpiryInMinutes} minutes.`, + ); + } this.#maxIngressExpiryInMinutes = options.ingressExpiryInMinutes || 5; From cf742f647f48f16e1c5a3b4541e05d3fa683cba4 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Thu, 18 Jul 2024 12:14:54 -0700 Subject: [PATCH 3/4] feat: Expiry rounds to nearest second if delta is under 90 seconds --- e2e/node/basic/mainnet.test.ts | 4 ++-- e2e/node/integration/actor.test.ts | 4 ++-- packages/agent/src/actor.test.ts | 4 ++-- packages/agent/src/agent/http/transforms.test.ts | 13 +++++++++++++ packages/agent/src/agent/http/transforms.ts | 11 ++++++++++- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/e2e/node/basic/mainnet.test.ts b/e2e/node/basic/mainnet.test.ts index fd844f6a..c2d9b601 100644 --- a/e2e/node/basic/mainnet.test.ts +++ b/e2e/node/basic/mainnet.test.ts @@ -142,7 +142,7 @@ describe('call forwarding', () => { }); // TODO: change Expiry logic rounding to make <= 1 minute expiry work -test.skip('it should succeed when setting an expiry in the near future', async () => { +test('it should succeed when setting an expiry in the near future', async () => { ``; const agent = await HttpAgent.create({ host: 'https://icp-api.io', @@ -157,7 +157,7 @@ test.skip('it should succeed when setting an expiry in the near future', async ( arg: fromHex('4449444c0000'), effectiveCanisterId: 'tnnnb-2yaaa-aaaab-qaiiq-cai', }), - ).rejects.toThrowError(`Specified ingress_expiry not within expected range`); + ).resolves.toBeDefined(); }); test('it should succeed when setting an expiry in the future', async () => { diff --git a/e2e/node/integration/actor.test.ts b/e2e/node/integration/actor.test.ts index 49b48cf4..855dce28 100644 --- a/e2e/node/integration/actor.test.ts +++ b/e2e/node/integration/actor.test.ts @@ -14,9 +14,9 @@ test("Legacy Agent interface should be accepted by Actor's createActor", async ( ); // Verify that update calls work - await actor.write(8n); //? + await actor.write(8n); // Verify that query calls work - const count = await actor.read(); //? + const count = await actor.read(); expect(count).toBe(8n); }, 15_000); // TODO: tests for rejected, unknown time out diff --git a/packages/agent/src/actor.test.ts b/packages/agent/src/actor.test.ts index 8f3ef481..d0ef0cfe 100644 --- a/packages/agent/src/actor.test.ts +++ b/packages/agent/src/actor.test.ts @@ -292,7 +292,7 @@ describe('makeActor', () => { expect(reply).toEqual(canisterDecodedReturnValue); expect(replyUpdate).toEqual(canisterDecodedReturnValue); expect(replyWithHttpDetails.result).toEqual(canisterDecodedReturnValue); - replyWithHttpDetails.httpDetails['requestDetails']; //? + replyWithHttpDetails.httpDetails['requestDetails']; expect(replyWithHttpDetails.httpDetails).toMatchInlineSnapshot(` { "headers": [], @@ -330,7 +330,7 @@ describe('makeActor', () => { `); expect(replyUpdateWithHttpDetails.result).toEqual(canisterDecodedReturnValue); - replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); //? + replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); expect(replyUpdateWithHttpDetails.httpDetails).toMatchSnapshot(); }); diff --git a/packages/agent/src/agent/http/transforms.test.ts b/packages/agent/src/agent/http/transforms.test.ts index 7521309c..a4981976 100644 --- a/packages/agent/src/agent/http/transforms.test.ts +++ b/packages/agent/src/agent/http/transforms.test.ts @@ -13,3 +13,16 @@ test('it should round down to the nearest minute', () => { ).toISOString(); expect(expiry_as_date_string).toBe('2021-04-26T17:51:00.000Z'); }); + +test('it should round down to the nearest second if less than 90 seconds', () => { + // 2021-04-26T17:47:11.314Z - high precision + jest.setSystemTime(new Date(1619459231314)); + + const expiry = new Expiry(89 * 1000); + expect(expiry['_value']).toEqual(BigInt(1619459320000000000n)); + + const expiry_as_date_string = new Date( + Number(expiry['_value'] / BigInt(1_000_000)), + ).toISOString(); + expect(expiry_as_date_string).toBe('2021-04-26T17:48:40.000Z'); +}); diff --git a/packages/agent/src/agent/http/transforms.ts b/packages/agent/src/agent/http/transforms.ts index 934dfbfb..cc3b58d3 100644 --- a/packages/agent/src/agent/http/transforms.ts +++ b/packages/agent/src/agent/http/transforms.ts @@ -17,12 +17,21 @@ export class Expiry { private readonly _value: bigint; constructor(deltaInMSec: number) { + // if ingress as seconds is less than 90, round to nearest second + if (deltaInMSec < 90 * 1_000) { + // Raw value without subtraction of REPLICA_PERMITTED_DRIFT_MILLISECONDS + const raw_value = BigInt(Date.now() + deltaInMSec) * NANOSECONDS_PER_MILLISECONDS; + const ingress_as_seconds = raw_value / BigInt(1_000_000_000); + this._value = ingress_as_seconds * BigInt(1_000_000_000); + return; + } + // Use bigint because it can overflow the maximum number allowed in a double float. const raw_value = BigInt(Math.floor(Date.now() + deltaInMSec - REPLICA_PERMITTED_DRIFT_MILLISECONDS)) * NANOSECONDS_PER_MILLISECONDS; - // round down to the nearest second + // round down to the nearest second (since ) const ingress_as_seconds = raw_value / BigInt(1_000_000_000); // round down to nearest minute From 90f3bbdbde0468668b6c7f35dd3517b2aadef4fc Mon Sep 17 00:00:00 2001 From: Jason I Date: Tue, 22 Oct 2024 18:02:16 -0700 Subject: [PATCH 4/4] chore: fix bad syntax --- e2e/node/basic/mainnet.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/e2e/node/basic/mainnet.test.ts b/e2e/node/basic/mainnet.test.ts index 573a9aad..001aa01d 100644 --- a/e2e/node/basic/mainnet.test.ts +++ b/e2e/node/basic/mainnet.test.ts @@ -144,7 +144,6 @@ describe('call forwarding', () => { // TODO: change Expiry logic rounding to make <= 1 minute expiry work test('it should succeed when setting an expiry in the near future', async () => { - ``; const agent = await HttpAgent.create({ host: 'https://icp-api.io', ingressExpiryInMinutes: 1, @@ -162,7 +161,6 @@ test('it should succeed when setting an expiry in the near future', async () => }); test('it should succeed when setting an expiry in the future', async () => { - ``; const agent = await HttpAgent.create({ host: 'https://icp-api.io', ingressExpiryInMinutes: 5, @@ -186,6 +184,7 @@ test('it should fail when setting an expiry in the far future', async () => { ingressExpiryInMinutes: 100, }), ).rejects.toThrowError(`The maximum ingress expiry time is 5 minutes`); +}); test('it should allow you to set an incorrect root key', async () => { const agent = HttpAgent.createSync({