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: call returns raw certificate, if requested #877

Closed

Conversation

seniorjoinu
Copy link

Description

It is possible to speed up inter-canister calls in some scenarios, if delegate making the call and transferring the response to the user. For example, you've make an ICRC-1 token transfer to some dApp. This dApp, instead of making its own inter-canister call to token's index canister, would ask the user itself to fetch the block from the index and pass the certified response back to one of its own canisters, which can then verify the response.

Such an optimization requires being able to make a call and simultaneously retrieve the raw certificate client-side. This PR adds this feature in a clean non-breaking way.

Also, there was an accidentally discovered bug related to pollForResponse function. It was invoked with blsVerify parameter in place of request parameter. Maybe it did nothing, but it seemed suspicious. This wasn't catched by Typescript, because request parameter is of type any.

Note

  1. The certificate is only returned when CallConfig.returnCertificate is set to true. In that case, instead of just reply the method will return { result: reply, cert: rawCertificate }. If CallConfig.returnCertificate is unset of false - nothing changes, everything works as usual.
  2. The certificate is only returned for update calls. For query calls empty buffer is returned like this { result: reply, cert: emptyBuf }. This is done to: a) comply with method signature change (ActorMethod interface is the same for query and update calls); b) enable replica signature/certificate to be returned the same way for queries in future.
  3. I was not able to run e2e tests - could not find any scripts mentioned in the docs. Also, docs on local development seem outdated. Please, take action.

How Has This Been Tested?

Added a unit-test to actor.test.ts testcase.

fix: pollForResponse blsVerify passed in place of request

add CallConfig.returnCertificate parameter and typings
@seniorjoinu seniorjoinu requested a review from a team as a code owner April 26, 2024 14:08
@seniorjoinu
Copy link
Author

seniorjoinu commented Apr 26, 2024

Usage example

// returns the certificate
const { result, cert } = await canister.do_stuff.withOptions({ returnCertificate: true })(args);

// works as usual, if executed without options
const result = await canister.do_stuff();

// works as usual, if `returnCertificate` option is unset
const result1 = await canister.do_stuff.withOptions({ canisterId: cid })(args);

// works as usual, if `returnCertificate` option is `false`
const result3 = await canister.do_stuff.withOptions({ returnCertificate: false })(args);

// typescript will throw an error
const result = await canister.do_stuff.withOptions({ returnCertificate: true })(args);
processResult(result); // <- result is type { result: Result, cert: ArrayBuffer } and not just Result

// typescript will throw an error
const { result, cert } = await canister.do_stuff(); // <- type Result has no fields "cert", "result"

@seniorjoinu seniorjoinu changed the title feat[agent]: call returns raw certificate, if requested feat: call returns raw certificate, if requested Apr 26, 2024
@krpeacock
Copy link
Contributor

I'll need to think a little more about the design of modifying the return signature based on an option. I might want to take a slightly different approach to the API, but I do appreciate the work you've done on this

@seniorjoinu
Copy link
Author

seniorjoinu commented May 2, 2024

@krpeacock

I'll need to think a little more about the design of modifying the return signature based on an option. I might want to take a slightly different approach to the API, but I do appreciate the work you've done on this

We could also do this as a separate method, similar to .withOptions. Something like .andCertificate(). The reason why I didn't do this in the first place is because in that case we would probably need two of those: one regular and one with http details. This would be better for JS without TS setups.

I could implement that very quickly, just say your word. We need that feature for our project asap, so I would do whatever it takes.

Also, what do you think about the fixing part? Was there a bug in pollForResponse or not?

@seniorjoinu
Copy link
Author

@krpeacock pinging you once again.
Hope it is fine.

@seniorjoinu seniorjoinu deleted the feat-call-returns-raw-certificate branch May 17, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants