Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for proof of absence in certificate lookups #878

Merged
merged 3 commits into from
Jun 3, 2024
Merged

feat: add support for proof of absence in certificate lookups #878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f988a2c
feat: add support for proof of absence in certificate lookups
nathanosdev Apr 29, 2024
2ef1692
feat: propogate lookup result to public interface
nathanosdev May 7, 2024
bdee61b
Merge branch 'main' into nathan/add-proof-of-absence-support
krpeacock Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## [Unreleased]

### Added

- feat!: add support for proof of absence in Certificate lookups

### Changed

- fix: publish script will correctly update the `package-lock.json` file with the correct dependencies when making a new release
Expand Down
38 changes: 29 additions & 9 deletions e2e/node/basic/basic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { ActorMethod, Certificate, getManagementCanister } from '@dfinity/agent';
import {
ActorMethod,
Certificate,
LookupResultFound,
LookupStatus,
getManagementCanister,
} from '@dfinity/agent';
import { IDL } from '@dfinity/candid';
import { Principal } from '@dfinity/principal';
import agent from '../utils/agent';
Expand All @@ -18,14 +24,21 @@ test('read_state', async () => {
rootKey: resolvedAgent.rootKey,
canisterId: canisterId,
});
expect(cert.lookup([new TextEncoder().encode('Time')])).toBe(undefined);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const rawTime = cert.lookup(path)!;
expect(cert.lookup([new TextEncoder().encode('Time')])).toEqual({ status: LookupStatus.Unknown });

let rawTime = cert.lookup(path);

expect(rawTime.status).toEqual(LookupStatus.Found);
rawTime = rawTime as LookupResultFound;

expect(rawTime.value).toBeInstanceOf(ArrayBuffer);
rawTime.value = rawTime.value as ArrayBuffer;

const decoded = IDL.decode(
[IDL.Nat],
new Uint8Array([
...new TextEncoder().encode('DIDL\x00\x01\x7d'),
...(new Uint8Array(rawTime) || []),
...(new Uint8Array(rawTime.value) || []),
]),
)[0];
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -54,14 +67,21 @@ test('read_state with passed request', async () => {
rootKey: resolvedAgent.rootKey,
canisterId: canisterId,
});
expect(cert.lookup([new TextEncoder().encode('Time')])).toBe(undefined);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const rawTime = cert.lookup(path)!;
expect(cert.lookup([new TextEncoder().encode('Time')])).toEqual({ status: LookupStatus.Unknown });

let rawTime = cert.lookup(path);

expect(rawTime.status).toEqual(LookupStatus.Found);
rawTime = rawTime as LookupResultFound;

expect(rawTime.value).toBeInstanceOf(ArrayBuffer);
rawTime.value = rawTime.value as ArrayBuffer;

const decoded = IDL.decode(
[IDL.Nat],
new Uint8Array([
...new TextEncoder().encode('DIDL\x00\x01\x7d'),
...(new Uint8Array(rawTime) || []),
...(new Uint8Array(rawTime.value) || []),
]),
)[0];
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
13 changes: 9 additions & 4 deletions packages/agent/src/agent/http/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ import {
} from './types';
import { AgentHTTPResponseError } from './errors';
import { SubnetStatus, request } from '../../canisterStatus';
import { CertificateVerificationError, HashTree, lookup_path } from '../../certificate';
import {
CertificateVerificationError,
HashTree,
LookupStatus,
lookup_path,
} from '../../certificate';
import { ed25519 } from '@noble/curves/ed25519';
import { ExpirableMap } from '../../utils/expirableMap';
import { Ed25519PublicKey } from '../../public_key';
Expand Down Expand Up @@ -902,14 +907,14 @@ export class HttpAgent implements Agent {
throw new Error('Could not decode time from response');
}
const timeLookup = lookup_path(['time'], tree);
if (!timeLookup) {
if (timeLookup.status !== LookupStatus.Found) {
throw new Error('Time was not found in the response or was not in its expected format.');
}

if (!(timeLookup instanceof ArrayBuffer) && !ArrayBuffer.isView(timeLookup)) {
if (!(timeLookup.value instanceof ArrayBuffer) && !ArrayBuffer.isView(timeLookup)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your changes, but would be worthwhile simplifying this check / making it more robust using the bufFromBufLike util up here instead of relying on instanceof

Copy link
Member Author

@nathanosdev nathanosdev May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timelookup.value is either an ArrayBuffer or a HashTree. So if we pass timelookup.value into bufFromBufLike when timelookup.value is a HashTree, it will pass the Array.isArray test in bufFromBuflike and be returned as an ArrayBuffer. This ArrayBuffer will then be passed to decodeTime which may actually succeed in decoding a timestamp from it, depending on the format of the HashTree. This results in returning an incorrect timestamp when we shouldn't.

It's more robust to exist early if we receive a malformed response in my opinion.

throw new Error('Time was not found in the response or was not in its expected format.');
}
const date = decodeTime(bufFromBufLike(timeLookup));
const date = decodeTime(bufFromBufLike(timeLookup.value as ArrayBuffer));
this.log('Time from response:', date);
this.log('Time from response in milliseconds:', Number(date));
return Number(date);
Expand Down
24 changes: 18 additions & 6 deletions packages/agent/src/canisterStatus/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
HashTree,
flatten_forks,
check_canister_ranges,
lookupResultToBuffer,
LookupStatus,
lookup_path,
lookupResultToBuffer,
} from '../certificate';
import { toHex } from '../utils/buffer';
import * as Cbor from '../cbor';
Expand Down Expand Up @@ -290,14 +291,25 @@ export const fetchNodeKeys = (
throw new Error('Canister not in range');
}

const nodeTree = lookup_path(['subnet', delegation?.subnet_id as ArrayBuffer, 'node'], tree);
const nodeForks = flatten_forks(nodeTree as HashTree) as HashTree[];
nodeForks.length;
const subnetLookupResult = lookup_path(['subnet', delegation.subnet_id, 'node'], tree);
if (subnetLookupResult.status !== LookupStatus.Found) {
throw new Error('Node not found');
}
if (subnetLookupResult.value instanceof ArrayBuffer) {
throw new Error('Invalid node tree');
}

const nodeForks = flatten_forks(subnetLookupResult.value);
const nodeKeys = new Map<string, DerEncodedPublicKey>();

nodeForks.forEach(fork => {
Object.getPrototypeOf(new Uint8Array(fork[1] as ArrayBuffer));
const node_id = Principal.from(new Uint8Array(fork[1] as ArrayBuffer)).toText();
const derEncodedPublicKey = lookup_path(['public_key'], fork[2] as HashTree) as ArrayBuffer;
const publicKeyLookupResult = lookup_path(['public_key'], fork[2] as HashTree);
if (publicKeyLookupResult.status !== LookupStatus.Found) {
throw new Error('Public key not found');
}

const derEncodedPublicKey = publicKeyLookupResult.value as ArrayBuffer;
if (derEncodedPublicKey.byteLength !== 44) {
throw new Error('Invalid public key length');
} else {
Expand Down
Loading
Loading