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.

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: retry query signature verification in case cache is stale #801

Merged
merged 6 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/generated/changelog.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ <h1>Agent-JS Changelog</h1>

<section>
<h2>Version x.x.x</h2>
<ul>
<li>feat: retry query signature verification in case cache is stale</li>
</ul>
<h2>Version 0.20.0</h2>
<ul>
<li>feat: uses expirable map for subnet keys in agent-js, with a timeout of 1 hour</li>
<li>chore: cleanup for node 20 development in agent-js</li>
Expand Down
2 changes: 2 additions & 0 deletions packages/agent/src/agent/http/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -790,3 +790,5 @@ test('retry requests that fail due to a network failure', async () => {
expect(mockFetch.mock.calls.length).toBe(4);
}
});

test.todo('retry query signature validation after refreshing the subnet node keys');
25 changes: 20 additions & 5 deletions packages/agent/src/agent/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export class HttpAgent implements Agent {
#updatePipeline: HttpAgentRequestTransformFn[] = [];

#subnetKeys: ExpirableMap<string, SubnetStatus> = new ExpirableMap({
expirationTime: 60 * 60 * 1000, // 1 hour
expirationTime: 5 * 60 * 1000, // 5 minutes
});
#verifyQuerySignatures = true;

Expand Down Expand Up @@ -532,7 +532,22 @@ export class HttpAgent implements Agent {
if (!this.#verifyQuerySignatures) {
return query;
}
return this.#verifyQueryResponse(query, subnetStatus);
try {
return this.#verifyQueryResponse(query, subnetStatus);
} catch (_) {
// In case the node signatures have changed, refresh the subnet keys and try again
console.warn('Query response verification failed. Retrying with fresh subnet keys.');
this.#subnetKeys.delete(canisterId.toString());
await this.fetchSubnetKeys(canisterId.toString());

const updatedSubnetStatus = this.#subnetKeys.get(canisterId.toString());
if (!updatedSubnetStatus) {
throw new CertificateVerificationError(
'Invalid signature from replica signed query: no matching node key found.',
);
}
return this.#verifyQueryResponse(query, updatedSubnetStatus);

Choose a reason for hiding this comment

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

Is it okay that there is no resolve(subnetStatus); call here, in between get and verifyQueryResponse, as in the above code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we are not within a Promise at this stage in the code (apart from the async query function itself). It's a little semantically incongruous if you're comparing this section to the above sections, however

}
}

/**
Expand All @@ -554,10 +569,10 @@ export class HttpAgent implements Agent {
'Invalid signature from replica signed query: no matching node key found.',
);
}
const { status, signatures, requestId } = queryResponse;
const { status, signatures = [], requestId } = queryResponse;

const domainSeparator = new TextEncoder().encode('\x0Bic-response');
signatures?.forEach(sig => {
for (const sig of signatures) {
const { timestamp, identity } = sig;
const nodeId = Principal.fromUint8Array(identity).toText();
let hash: ArrayBuffer;
Expand Down Expand Up @@ -605,7 +620,7 @@ export class HttpAgent implements Agent {
throw new CertificateVerificationError(
`Invalid signature from replica ${nodeId} signed query.`,
);
});
}
return queryResponse;
};

Expand Down
Loading