-
Notifications
You must be signed in to change notification settings - Fork 96
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!: support getting certificate back from call #892
Conversation
size-limit report 📦
|
@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 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 You can inspect any nonces as well as the ingress_expiry, as these are now returned by |
@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! |
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.
Thanks a lot, this is what we need. 👍
@peterpeterparker: Want to take a look as well?
Related to this, on my end with Current workaround in 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 😅 |
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 I need 4 items for that:
Regarding the certificate, everything seems fine in the PR. What's missing:
@frederikrothenberger, are there better ways to achieve these three objectives without changing agent.js? Shall we consider the PR to cover all the cases? |
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 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. |
@sea-snake We've always had the cbor implementation exported - import { Cbor } from '@dfinity/agent'; Am I missing something there? |
Wasn't aware of this export, great to know! |
…nto kai/SDK-1717-raw-call
@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:
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.
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 |
@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 |
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 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. |
…nto kai/SDK-1717-raw-call
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:In addition, the output from
pollForResponse
needs to be updated as well. PollForResponse now returnsThis will be a breaking change, requiring a major version bump.
Checklist: