-
Notifications
You must be signed in to change notification settings - Fork 98
Add ability to make an actor from an interface #79
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
Changes from 3 commits
15ccabe
b9f7ee8
21554af
8cf74de
24ae4cf
db0289d
7dbf96c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { Buffer } from "buffer/"; | ||
|
|
||
| export interface Func { | ||
| argTypes: Array<Type>; | ||
| retTypes: Array<Type>; | ||
| } | ||
|
|
||
| interface JsArray extends Array<JsValue> {} | ||
| type JsValue = boolean | string | number | JsArray | object | ||
|
|
||
| export interface Type { | ||
| encode(x: JsValue): Buffer | ||
| decode(x: Buffer): JsValue | ||
| } | ||
|
|
||
| export interface Text extends Type {} | ||
|
|
||
| export class ActorInterface { | ||
| __fields: object | ||
| constructor(fields: object) | ||
| } | ||
|
|
||
| export function idlHash(s: string): number | ||
|
|
||
| export function Func(argTypes?: Array<Type>, retTypes?: Array<Type>): Func | ||
| export const Text: Text | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| import { BinaryBlob } from "./blob"; | ||
| import * as blob from "./blob"; | ||
| import * as canisterId from "./canisterId"; | ||
| import * as cbor from "./cbor"; | ||
| import { Hex } from "./hex"; | ||
| import { Nonce } from "./nonce"; | ||
| import { Request } from "./request"; | ||
| import { requestIdOf } from "./requestId"; | ||
| import { SenderPubKey } from "./senderPubKey"; | ||
| import { SenderSig } from "./senderSig"; | ||
|
|
||
| import { | ||
| IDL as _IDL, | ||
| makeActor, | ||
| makeHttpAgent, | ||
| } from "./index"; | ||
|
|
||
| test("makeActor", async () => { | ||
| const actorInterface = ({ IDL }: { IDL: typeof _IDL }) => { | ||
| return new IDL.ActorInterface({ | ||
| greet: IDL.Func([IDL.Text], [IDL.Text]), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chenyan-dfinity above is the format we used in the produce exchange, but I see the following format used in the IDL tests: Are these supposed to be equivalent? If so I think it has implications for encoding and decoding, when we get to that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I will fix the IDL codegen. This is a hack with the old system API. |
||
| }); | ||
| }; | ||
|
|
||
| const expectedReplyArg = new Uint8Array( | ||
| _IDL.Text.encode("Hello, World!").buffer, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They're the same thing. On line 13 |
||
| ) as BinaryBlob; | ||
|
|
||
| const mockFetch: jest.Mock = jest.fn() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For future PR: let's try to add a test where we handle errors. |
||
| .mockImplementationOnce((/*resource, init*/) => { | ||
| return Promise.resolve(new Response(null, { | ||
| status: 202, | ||
| })); | ||
| }) | ||
| .mockImplementationOnce((resource, init) => { | ||
| const body = cbor.encode({ status: "unknown" }); | ||
| return Promise.resolve(new Response(body, { | ||
| status: 200, | ||
| })); | ||
| }) | ||
| .mockImplementationOnce((resource, init) => { | ||
| const body = cbor.encode({ status: "pending" }); | ||
| return Promise.resolve(new Response(body, { | ||
| status: 200, | ||
| })); | ||
| }) | ||
| .mockImplementationOnce((resource, init) => { | ||
| const body = cbor.encode({ | ||
| status: "replied", | ||
| reply: { | ||
| arg: expectedReplyArg, | ||
| }, | ||
| }); | ||
| return Promise.resolve(new Response(body, { | ||
| status: 200, | ||
| })); | ||
| }); | ||
|
|
||
| const methodName = "greet"; | ||
| const arg = Uint8Array.from([]); | ||
|
|
||
| const canisterIdent = "0000000000000001" as Hex; | ||
| const senderPubKey = new Uint8Array(32) as SenderPubKey; | ||
| const senderSig = new Uint8Array(64) as SenderSig; | ||
|
|
||
| const nonces = [ | ||
| Uint8Array.from([0, 1, 2, 3, 4, 5, 6, 7]) as Nonce, | ||
| Uint8Array.from([1, 2, 3, 4, 5, 6, 7, 8]) as Nonce, | ||
| Uint8Array.from([2, 3, 4, 5, 6, 7, 8, 9]) as Nonce, | ||
| Uint8Array.from([3, 4, 5, 6, 7, 8, 9, 0]) as Nonce, | ||
| ]; | ||
|
|
||
| const expectedCallRequest = { | ||
| request_type: "call", | ||
| nonce: nonces[0], | ||
| canister_id: canisterId.fromHex(canisterIdent), | ||
| method_name: methodName, | ||
| arg, | ||
| sender_pubkey: senderPubKey, | ||
| sender_sig: senderSig, | ||
| } as Request; | ||
|
|
||
| const expectedCallRequestId = await requestIdOf(expectedCallRequest); | ||
|
|
||
| let nonceCount = 0; | ||
|
|
||
| const httpAgent = makeHttpAgent({ | ||
| canisterId: canisterIdent, | ||
| fetchFn: mockFetch, | ||
| nonceFn: () => { | ||
| const nonce = nonces[nonceCount]; | ||
| nonceCount = nonceCount + 1; | ||
| return nonce; | ||
| }, | ||
| senderPubKey, | ||
| senderSigFn: () => senderSig, | ||
| }); | ||
|
|
||
| const actor = makeActor(actorInterface)(httpAgent); | ||
| // FIXME: the argument isn't actually used yet | ||
| const reply = await actor.greet("Name"); | ||
|
|
||
| expect( | ||
| blob.toHex(reply), | ||
| ).toEqual( | ||
| blob.toHex(expectedReplyArg), | ||
| ); | ||
|
|
||
| const { calls, results } = mockFetch.mock; | ||
| expect(calls.length).toBe(4); | ||
|
|
||
| expect(calls[0][0]).toBe("http://localhost:8000/api/v1/submit"); | ||
| expect(calls[0][1]).toEqual({ | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/cbor", | ||
| }, | ||
| body: cbor.encode(expectedCallRequest), | ||
| }); | ||
|
|
||
| expect(calls[1][0]).toBe("http://localhost:8000/api/v1/read"); | ||
| expect(calls[1][1]).toEqual({ | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/cbor", | ||
| }, | ||
| body: cbor.encode({ | ||
| request_type: "request-status", | ||
| nonce: nonces[1], | ||
| request_id: expectedCallRequestId, | ||
| sender_pubkey: senderPubKey, | ||
| sender_sig: senderSig, | ||
| }), | ||
| }); | ||
|
|
||
| expect(calls[2][0]).toBe("http://localhost:8000/api/v1/read"); | ||
| expect(calls[2][1]).toEqual({ | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/cbor", | ||
| }, | ||
| body: cbor.encode({ | ||
| request_type: "request-status", | ||
| nonce: nonces[2], | ||
| request_id: expectedCallRequestId, | ||
| sender_pubkey: senderPubKey, | ||
| sender_sig: senderSig, | ||
| }), | ||
| }); | ||
|
|
||
| expect(calls[3][0]).toBe("http://localhost:8000/api/v1/read"); | ||
| expect(calls[3][1]).toEqual({ | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/cbor", | ||
| }, | ||
| body: cbor.encode({ | ||
| request_type: "request-status", | ||
| nonce: nonces[3], | ||
| request_id: expectedCallRequestId, | ||
| sender_pubkey: senderPubKey, | ||
| sender_sig: senderSig, | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| // TODO: tests for rejected, unknown time out | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,88 @@ | ||||||
| import { BinaryBlob } from "./blob"; | ||||||
| import { HttpAgent } from "./httpAgent"; | ||||||
| import _IDL from "./IDL"; | ||||||
| import * as requestId from "./requestId"; | ||||||
|
|
||||||
| import { | ||||||
| RequestStatusResponse, | ||||||
| RequestStatusResponseStatus, | ||||||
| } from "./requestStatusResponse"; | ||||||
|
|
||||||
| import retry from "async-retry"; | ||||||
|
|
||||||
| // Make an actor from an actor interface. | ||||||
| // | ||||||
| // Allows for one HTTP agent for the lifetime of the actor: | ||||||
| // | ||||||
| // ``` | ||||||
| // const actor = makeActor(actorInterface)(httpAgent); | ||||||
| // const reply = await actor.greet(); | ||||||
| // ``` | ||||||
| // | ||||||
| // or using a different HTTP agent for the same actor if necessary: | ||||||
| // | ||||||
| // ``` | ||||||
| // const actor = makeActor(actorInterface); | ||||||
| // const reply1 = await actor(httpAgent1).greet(); | ||||||
| // const reply2 = await actor(httpAgent2).greet(); | ||||||
| // ``` | ||||||
| export const makeActor = ( | ||||||
| makeActorInterface: ({ IDL }: { IDL: typeof _IDL }) => _IDL.ActorInterface, | ||||||
| ) => ( | ||||||
| httpAgent: HttpAgent, | ||||||
| ): Record<string, (...args: Array<any>) => any> => { | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may not be obvious so I want to draw attention to it. The way this currently works is that At present, I think the important thing to note here is that what the IDL compiler produces aren't really "bindings". They weren't actually bindings before I made that PR, but arguably they had the potential to be because they imported the user library (sort of, see the PR linked above). Now the user library imports what the IDL compiler generates, which is a JS representation of the actor's interface. I think this makes a lot more sense given the information required to communicate with a client, such as canister IDs, entity IDs, etc
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the context. Yes, I am missing some related context here, so will continue in the morning.
paulyoung marked this conversation as resolved.
|
||||||
| const actorInterface = makeActorInterface({ IDL: _IDL }); | ||||||
| const entries = Object.entries(actorInterface.__fields); | ||||||
| return Object.fromEntries(entries.map((entry) => { | ||||||
| const [methodName, func] = entry as [string, _IDL.Func]; | ||||||
| return [methodName, async (...args: Array<any>) => { | ||||||
| // TODO | ||||||
| // * Throw if func.argTypes.length !== args.length | ||||||
| // * Encode arguments | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| const { | ||||||
| requestId: requestIdent, | ||||||
| response: callResponse, | ||||||
| } = await httpAgent.call({ | ||||||
| methodName, | ||||||
| arg: Uint8Array.from([]) as BinaryBlob, | ||||||
| }); | ||||||
|
|
||||||
| if (!callResponse.ok) { | ||||||
| throw new Error([ | ||||||
| `Request failed:`, | ||||||
| ` Request ID: ${requestId.toHex(requestIdent)}`, | ||||||
| ` HTTP status code: ${callResponse.status}`, | ||||||
| ` HTTP status text: ${callResponse.statusText}`, | ||||||
| ].join("\n")); | ||||||
| } | ||||||
|
|
||||||
| const maxAttempts = 3; | ||||||
|
|
||||||
| const reply = await retry(async (bail, attempts) => { | ||||||
| const response: RequestStatusResponse = await httpAgent.requestStatus({ | ||||||
| requestId: requestIdent, | ||||||
| }); | ||||||
|
|
||||||
| switch (response.status) { | ||||||
| case RequestStatusResponseStatus.Replied: { | ||||||
| // TODO | ||||||
| // * Throw if func.retTypes.length !== response.reply.arg.length | ||||||
| // * Decode response | ||||||
| return response.reply.arg; | ||||||
| } | ||||||
| default: { | ||||||
| throw new Error([ | ||||||
| `Failed to retrieve a reply for request after ${attempts} attempts:`, | ||||||
| ` Request ID: ${requestId.toHex(requestIdent)}`, | ||||||
| ` Request status: ${response.status}`, | ||||||
| ].join("\n")); | ||||||
| } | ||||||
| } | ||||||
| }, { | ||||||
| retries: maxAttempts - 1, | ||||||
| }); | ||||||
|
|
||||||
| return reply; | ||||||
| }]; | ||||||
| })); | ||||||
| }; | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,5 @@ | ||
| export { makeActor } from "./actor"; | ||
| export { HttpAgent, makeHttpAgent } from "./httpAgent"; | ||
|
|
||
| import * as IDL from "./IDL"; | ||
| export { IDL }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough JS, but why do we want to export
Text, not other IDL types?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I just added the minimal definitions to get a demo working.