dfinity / agent-js

A collection of libraries and tools for building software around the Internet Computer, in JavaScript.
https://agent-js.icp.xyz
Apache License 2.0
153 stars 95 forks source link

feat!: support getting certificate back from call #892

Closed krpeacock closed 3 months ago

krpeacock commented 4 months ago

Description

In order to support ICRC-49, we need the HttpAgent to provide more details back after making a call. With this change, HttpAgent.call will provide:

requestId - the computed request ID to poll for
response - the raw `http` response from the boundary node
requestDetails - the details sent to the canister, used to compute the request ID

In addition, the output from pollForResponse needs to be updated as well. PollForResponse now returns

certificate: the Certificate tree sent along with the reply
reply: the certified response from the replica

This will be a breaking change, requiring a major version bump.

Checklist:

github-actions[bot] commented 4 months ago

size-limit report 📦

Path Size
@dfinity/agent 85.94 KB (+0.21% 🔺)
@dfinity/candid 13.58 KB (0%)
@dfinity/principal 4.97 KB (0%)
@dfinity/auth-client 60.8 KB (0%)
@dfinity/assets 80.6 KB (+0.16% 🔺)
@dfinity/identity 58 KB (0%)
@dfinity/identity-secp256k1 266.45 KB (+0.12% 🔺)
krpeacock commented 4 months ago

@frederikrothenberger @peterpeterparker with these changes, you can forward a call without being aware of the arguments using code that would look something like this:

  const forwardedOptions = {
    canisterId: 'tnnnb-2yaaa-aaaab-qaiiq-cai',
    methodName: 'inc_read',
    arg: '4449444c0000',
    effectiveCanisterId: 'tnnnb-2yaaa-aaaab-qaiiq-cai',
  };

  const agent = new HttpAgent({ host: 'https://icp-api.io', fetch: global.fetch });
  const { requestId, response, requestDetails } = await agent.call(
    Principal.fromText(forwardedOptions.canisterId),
    {
      methodName: forwardedOptions.methodName,
      arg: fromHex(forwardedOptions.arg),
      effectiveCanisterId: Principal.fromText(forwardedOptions.effectiveCanisterId),
    },
  );

  const { certificate, reply } = await pollForResponse(
    agent,
    Principal.fromText(forwardedOptions.effectiveCanisterId),
    requestId,
    defaultStrategy(),
  );
  certificate; // Certificate
  reply; // ArrayBuffer

Should I wrap all of this functionality into a callRaw method, or does this satisfy your requirements?

Nonce generation is not used in calls and is disabled by default for queries with the agent now, but you will be able to control it through the useQueryNonces setting or adding transforms to the agent.

You can inspect any nonces as well as the ingress_expiry, as these are now returned by agent.call inside of requestDetails

frederikrothenberger commented 4 months ago

@krpeacock: Yes, this satisfies our requirement. Thanks a lot.

@dostro: Please share this with your team / compare to your fork and also provide feedback. Thanks!

sea-snake commented 4 months ago

Related to this, on my end with signer-js, I need to create a CallRequest instance but have to work around the fact that I can't create an Expiry instance with a specific value since this value is private and the constructor adds the current time to the passed argument.

Current workaround in signer-js:

const expiry = new Expiry(0);
// @ts-ignore Expiry class currently has no method to create instance from value
expiry._value = /* Some BigInt value */

Would be nice to be able to create an Expiry instance with a specific value or simply be able to modify it's value 😅

dmitriiIdentityLabs commented 3 months ago

Thank you, @frederikrothenberger, for letting us know about the PR!

Context: While implementing the ICRC-49 call canister standard, I faced challenges making the canister call on behalf of the user with the current agent.js implementation. To address this, I created a new actor, similar to ActorWithHttpDetails, called CallCanisterActor. Its sole purpose is to be useful for signer apps.

I need 4 items for that:

  1. Certificate.
  2. ContentMap.
  3. Update call enforcement.
  4. Ability to pass candid-encoded arguments to the actor.

Regarding the certificate, everything seems fine in the PR.

What's missing:

  1. There is no ContentMap as an output. I return contentMap: transformedRequest.body.content in packages/agent/src/agent/http/index.ts. Potentially, I can rebuild it outside of agent.js, but it requires the application of all inner transformations.
  2. The signer calls are always update calls, even if they are queries in Candid. To achieve this, I enforce agent.call if I have my option in the annotations: func.annotations.includes('call-canister').
  3. The signer call arguments are always candid-encoded before the actor method. There is no need to encode them in an actor. Therefore, I implement a different workflow to prevent the encoding of the argument if I have my option in the annotation: func.annotations.includes('call-canister').

@frederikrothenberger, are there better ways to achieve these three objectives without changing agent.js? Shall we consider the PR to cover all the cases?

sea-snake commented 3 months ago

To address this, I created a new actor, similar to ActorWithHttpDetails, called CallCanisterActor.

Why would you need an actor to make a call with raw arguments?

To clarify, the HttpAgent allows you to make communicate with canister methods annotated as either call, query (or composite-query). While an actor additionally simplifies things by using the canister candid definition to automatically create a class with methods for all canister methods defined in candid. An actor also automatically decides if it should use either a call or make a query for each candid method, automatically converts JS arguments into a single binary argument etc.

In the Signer standards, the decision was made to always call canister methods even if they could be queried instead. This decision was made due to various security aspects. Also the received arguments are already encoded by the relying party, so there's no need for an Actor to encode anything.

In practice this means that you don't need to create an Actor to make a ICRC-49 canister call since:

Given the incoming JSON-RPC message:

{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "icrc49_call_canister",
    "params": {
        "canisterId": "xhy27-fqaaa-aaaao-a2hlq-ca",
        "sender": "b7gqo-ulk5n-2kpo7-oalt7-p2kyl-o4j5l-kiuwo-eeybr-dab4l-ur6up-pqe",
        "method": "transfer",
        "arg": "RElETARte24AbAKzsNrDA2ithsqDBQFs5ofQIAAMgB"
    }
}

The signer could use below code to call the canister and return a JSON-RPC response:

import { HttpAgent, Cbor, polling} from '@dfinity/agent';
import { Buffer } from 'buffer';

const { params } = rpcRequest; // The JSON-RPC request from above
const agent = new HttpAgent();
const { pollForResponse, defaultStrategy } = polling;
const { requestId, requestDetails } = await agent.call(
    Principal.fromText(params.canisterId),
    {
      methodName: params.methodName,
      arg: Buffer.from(params.arg, 'base64').buffer,
    },
);

const contentMap = Cbor.encode(requestDetails);
const { certificate } = await pollForResponse(
    agent,
    Principal.fromText(params.canisterId),
    requestId,
    defaultStrategy(),
);

// JSON-RPC response for above JSON-RPC request
const rpcResponse = {
    id: rpcRequest.id,
    jsonrpc: '2.0',
    result: {
       contentMap: Buffer.from(contentMap).toString('base64'),
       certificate: Buffer.from(certificate).toString('base64'),
    }
};

@krpeacock Not having the cbor exported and available from @dfinity/agent or another lib is cumbersome though, currently I have to manually copy its implementation into my own project.

If you want to decode the already encoded binary argument and visualize it in the signer, that's a whole different story. You'd need to fetch the candid from the canister metadata and use that to decode the binary call argument into JS variables and then visualize this JS by e.g. rendering a tree. Overall, ideally you'd like to avoid the need for this and rely on ICRC-21 instead for human readable information to be presented to the end user.

krpeacock commented 3 months ago

@sea-snake We've always had the cbor implementation exported -

import { Cbor } from '@dfinity/agent';

Am I missing something there?

sea-snake commented 3 months ago

Am I missing something there?

Wasn't aware of this export, great to know!

dmitriiIdentityLabs commented 3 months ago

@sea-snake Thank you for the excellent example! I'll use it if we decide not to change the actor. The main issue is that the signer has to reuse a lot of code from the actor for polling, and encoding. This code could change, meaning every signer would need to implement it, placing the burden of compliance on the signer's shoulders.

The ideal scenario for me would be:

const actor = SignerActor.create(candidjs, {agent, canisterId});
const {contentMap, certificate} = await actor[method](encodedArg);

Why would you need an actor to make a call with raw arguments?

There's a way to avoid reimplementing the actor and copying its code. We could create a specific SignerActor class designed solely for signer call canisters, adhering strictly to the standards.

If you want to decode the already encoded binary argument and visualize it in the signer, that's a whole different story. You'd need to fetch the candid from the canister metadata and use that to decode the binary call argument into JS variables and then visualize this JS by e.g. rendering a tree. Overall, ideally you'd like to avoid the need for this and rely on ICRC-21 instead for human readable information to be presented to the end user.

Our example signer already implements this. We fetch the candid to decode arguments and display them to the user. I understand your point, but I worry we cannot fully rely on ICRC-21 for displaying human-readable data to users. The standard might not be implemented, and the signer might require additional validation for known call canister requests. Therefore, the signer must understand the data. It would also be beneficial to simplify the signer's task by providing a default way to fetch and process the candid dynamically.

@frederikrothenberger @sea-snake @krpeacock The main question is: Is there any chance we could consider adding a specific actor for call canister to agent-js, or would it be better to postpone this?

krpeacock commented 3 months ago

@dmitriiIdentityLabs I can add that, but I think I'll address it in a separate PR. I'm fairly sure that this API with these breaking changes will give us all we need to build that actor, so it can be added in a future PR with just a minor version bump

sea-snake commented 3 months ago

The ideal scenario for me would be:

const actor = SignerActor.create(candidjs, {agent, canisterId});
const {contentMap, certificate} = await actor[method](encodedArg);

I would at least not call it an actor since it isn't an actor in comparison to what actors currently are (following the actor model).

Not sure what the candidjs argument is for, there's no candid involved in either the input or output (both are already encoded raw bytes.

Also keep in mind that the ICRC-49 spec is working with raw already encoded bytes as input/output intentionally, because technically nothing stops a canister from using something other than candid for it's input/output. So not relying on candid in ICRC-49 keeps it future compatible with any data format.