Skip to content

Commit

Permalink
Merge branch 'main' into kai/agent-error-typechecking
Browse files Browse the repository at this point in the history
# Conflicts:
#	docs/CHANGELOG.md
  • Loading branch information
dfx-json committed Oct 23, 2024
2 parents 7e3bb46 + 7c147b8 commit 00475fc
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 5 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Added

- feat: allow for setting HttpAgent ingress expiry using `ingressExpiryInMinutes` option

- feat: improved assertion options for agent errors using `prototype`, `name`, and `instanceof`

### Changed
Expand Down
43 changes: 43 additions & 0 deletions e2e/node/basic/mainnet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,49 @@ describe('call forwarding', () => {
}, 15_000);
});

// 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,
});

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 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`);
});

test('it should allow you to set an incorrect root key', async () => {
const agent = HttpAgent.createSync({
Expand Down
10 changes: 10 additions & 0 deletions packages/agent/src/agent/http/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,16 @@ test('it should log errors to console if the option is set', async () => {
await agent.syncTime();
});

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`);
});

test('it should handle calls against the ic-management canister that are rejected', async () => {
const identity = new AnonymousIdentity();
identity.getPrincipal().toString();
Expand Down
32 changes: 28 additions & 4 deletions packages/agent/src/agent/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ export enum RequestStatusResponseStatus {
Done = 'done',
}

const MINUTE_TO_MSECS = 60 * 1000;

// Root public key for the IC, encoded as hex
export const IC_ROOT_KEY =
'308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c05030201036100814' +
Expand Down Expand Up @@ -106,6 +108,12 @@ export interface HttpAgentOptions {
// time (will throw).
identity?: Identity | Promise<Identity>;

/**
* The maximum time a request can be delayed before being rejected.
* @default 5 minutes
*/
ingressExpiryInMinutes?: number;

credentials?: {
name: string;
password?: string;
Expand Down Expand Up @@ -248,6 +256,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;
Expand Down Expand Up @@ -310,6 +319,19 @@ 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.`,
);
}
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;

// Add a nonce transform to ensure calls are unique
this.addTransform('update', makeNonceTransform(makeNonce));
if (options.useQueryNonces) {
Expand Down Expand Up @@ -428,11 +450,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 = {
Expand Down Expand Up @@ -772,7 +796,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);
Expand Down Expand Up @@ -957,7 +981,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),
},
});

Expand Down
13 changes: 13 additions & 0 deletions packages/agent/src/agent/http/transforms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
11 changes: 10 additions & 1 deletion packages/agent/src/agent/http/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 00475fc

Please sign in to comment.