Canister query and call commands #39

hansl · 2019-09-20T19:51:28Z

Call will generate a random nonce for now, to avoid errors.

There is currently no way to request status. As well, it seems that any changes to the memory at least in an actorscript build from the Counter example (see dfinity/motoko#660).

dfx canister query 42 greet DFINITY will work fine with the builtin canister.
dfx canister call 42 greet DFINITY works as well with the builtin canister (though can't read the result).
dfx canister install 42 PATH_TO_WASM works and updates the canister with the new counter wasm (or any wasm built).
Any query or call to any method (either read or inc) from the canister will kill the client, and restart it with the builtin canister.

I don't know if it's a problem with the actorscript output or the execution environment, but AFAICT the CLI makes the right call according to the public spec.

dfx/src/commands/canister/call.rs

paulyoung · 2019-09-20T21:06:29Z

dfx/src/commands/canister/call.rs

+        client,
+        canister_id,
+        method_name.to_owned(),
+        arguments.map(|args| Blob(Vec::from(args[0]))),


Is there a possibility that args could be empty and args[0] be out of bounds index access?

No, but this isn't represented in typing. values_of() will return None if there is no "arguments", so it needs at least 1 to have some value.

dfx/src/commands/canister/query.rs

paulyoung · 2019-09-20T21:16:13Z

dfx/src/commands/canister/query.rs

+            reject_code,
+            reject_message,
+        }) => Err(DfxError::ClientError(reject_code, reject_message)),
+        Ok(ReadResponse::Unknown) => Err(DfxError::Unknown("Unknown response".to_owned())),


I don't think this is what we want to do here. The spec says:

Warning: Immediately after submitting a request, this may fail (e.g. return with unknown) even though the system is still working on accepting the request as pending.

This was from the call method. I only moved the same logic.

Yeah, it was wrong before as well

Actually Query cannot return UNKNOWN, so I don't know why that's there. That would be a panic!("this should not happen"). I'll leave it here with a note that we should remove this response from query() when moving to the ic_http_api

In another PR I suggested having different response status types because some have pending and unknown and others don't. I think that's the way to go.

paulyoung · 2019-09-20T21:19:04Z

dfx/src/lib/api_client.rs

+    canister_id: CanisterId,
+    method_name: String,
+    arg: Option<Blob>,
+) -> impl Future<Item = Option<Blob>, Error = DfxError> {


I think Item = Option<Blob> is incorrect since submit calls have no response body.

According to this it can return an arg.

Requests to the submit endpoint can't return anything. What "Reply fields" means (as opposed to "Response fields" for a request to the read endpoint) is what fields will be present in the reply field when you make a subsequent request-status request and the status is replied.

paulyoung

The return type of call still needs to be updated

paulyoung · 2019-09-23T18:15:48Z

dfx/src/lib/mod.rs

@@ -1,3 +1,5 @@
 pub mod api_client;
 pub mod env;
 pub mod error;
+
+pub type CanisterId = u64;


It would be great if we could use phantom_newtype from the DFINITY repo for things like this. I asked John a while ago and he said he'd think about it.

(No changes required here, just wanted to comment about it)

Oh that's nice. Dunno why we shouldn't make that a public crate even, there's plenty of projects that could use newtype categories like that.

This type is moved in my PR for the ic_http_api. I'll make it a new type there.

paulyoung · 2019-09-23T18:19:49Z

dfx/src/commands/call.rs

+                reject_code,
+                reject_message,
+            } => err(DfxError::ClientError(reject_code, reject_message)),
+            ReadResponse::Unknown => err(DfxError::Unknown("Unknown response".to_owned())),


I still think we should change this to avoid problems when polling (maybe just println!("Unknown response")?) but up to you whether to do it now.

I want to get rid of this command altogether. It probably shouldn't be part of the developer main experience, which should use the frontend to interact with the canister, or use lower level canister subcommands.

Removed the command in a commit.

paulyoung · 2019-09-24T17:55:32Z

dfx/src/lib/api_client.rs

@@ -108,6 +108,9 @@ fn random_blob() -> Blob {

 /// A read request. Intended to remain private in favor of exposing specialized
 /// functions like `query` instead.
+///
+/// TODO: filter the output of this function when moving to ic_http_api.
+/// For example, it should never return Unknown or Pending, per the spec.


To reiterate what I said earlier: #36 (comment)

I think we should have RequestStatusResponse and QueryResponse since the statuses for request-status and query are different now (maybe they always were and I implemented this incorrectly)

And move the is_number into it, with is_canister instead to allow for canister ID proper typing.

DEPRECATION: please use "cannister call" instead

@hansl

commit 7c5e71488df430b89de637a15b56814db196e752 Author: Islam El-Ashi <ielashi@users.noreply.github.com> Date: Sat Nov 7 00:30:05 2020 +0100 feat: DER-encode public key and principal ID when using Plain Authentication. (#48) BREAKING CHANGE If a developer relies on the inner work of SenderPubKey or manually instanciate Principal, the API changed. commit a254861ae4bfe608734c8c190e4033cfe39f047b Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:17:32 2020 -0800 chore(deps): bump node-fetch from 2.6.0 to 2.6.1 in /e2e/node (#49) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 694849109bc28fea80119388504127e889b36d15 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:17:14 2020 -0800 chore(deps-dev): bump node-fetch from 2.6.0 to 2.6.1 in /packages/agent (#50) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 88af47f79b5e9f3a5e08abbf0ce03b3140357fc0 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:16:53 2020 -0800 chore(deps-dev): bump node-fetch in /packages/bootstrap (#51) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit ba4b082e5795150176f7adad989c3613edceb85f Author: Hans <hans@larsen.online> Date: Thu Nov 5 12:49:59 2020 -0800 test: add ic-ref, ci e2e and lerna support to repo (#47) This refactors the repository to use lerna to manage its monorepo's packages. It also adds the e2e tests from the SDK repo that we previously had to remove. More e2e tests will be added later, but this unblocks working with DER and Identity work as we can now at least run some e2e tests now (instead of mocking the http connection). commit 8baa1cfc227d003c2806df1057b92009159df250 Author: Hans <hans@larsen.online> Date: Tue Nov 3 11:04:36 2020 -0800 feat: use anonymous principal by default in HttpAgent (#46) If the auth transform is missing and the principal is anonymous, use an anonymous auth transform. Otherwise, use the auth transform as normal. # BREAKING CHANGE Before, an auth transform would not be called if it was missing, now it results as an error if the principal is not anonymous. This was because just using the packet as is without putting it in an envelope was a bug and should never be sent to a replica anyway. Now this is explicit. commit 1a714e6c53f5e467112b964b71204b6d2eb545ef Author: Hans <hans@larsen.online> Date: Tue Nov 3 11:04:15 2020 -0800 feat: add a getPrincipal() method to Agents (#40) commit 324e856944adb7f79a2afa2cffbdb6d144b35ab4 Author: Hans <hans@larsen.online> Date: Tue Nov 3 08:49:44 2020 -0800 feat: add the canister ID to the window.ic object (#41) This can be useful to know the canister ID of the current frontend. commit 8680a2cf4994b45c6e00ffb0b5a12b379f5ff919 Author: Hans <hans@larsen.online> Date: Mon Nov 2 21:57:21 2020 -0800 feat: add a cache bust hash at the end of javascript output (#39) commit 9f2ed968b7cc9f4b854af5e23edca19d34124909 Author: Benjamin Goering <benjamin.goering@dfinity.org> Date: Tue Sep 22 15:28:49 2020 -0700 docs: polish docs/npm-publish.md per @hansl feedback (#37) via: pr feedback on dfinity/agent-js#36

@hansl

commit 19c28008db5fa1582da573088b17d0bdee9e8bc2 Author: Hans <hans@larsen.online> Date: Mon Nov 9 14:47:25 2020 -0800 revert: Remove lerna (PR #47) (#59) This removes lerna, but attempt to not remove the e2e tests and the new packages/files that were added after the last PR. commit 426a94f356bc4e27f50c8bec55071b82105adbd1 Author: Hans <hans@larsen.online> Date: Mon Nov 9 13:54:04 2020 -0800 feat: use anonymous for the first retrieve (#53) Also include some cleanup of the scripts that arent used, and add a diagram for the bootstrapping process. This does not affect the worker yet. Only the first retrieve of the index.js on localhost. BREAKING CHANGE If a canister has a retrieve() method that relies on Principal, the principal used will change. Co-authored-by: Benjamin Goering <benjamin.goering@dfinity.org> commit b795d8b34ce38dd35e1bcf2b80e33108cf74161d Author: Andrew Wylde <2126677+codelemur@users.noreply.github.com> Date: Mon Nov 9 09:41:13 2020 -0800 feat(idp): add identity-provider package to repository (#52) adds plain HTML/ts and test setup for project commit 7c5e71488df430b89de637a15b56814db196e752 Author: Islam El-Ashi <ielashi@users.noreply.github.com> Date: Sat Nov 7 00:30:05 2020 +0100 feat: DER-encode public key and principal ID when using Plain Authentication. (#48) BREAKING CHANGE If a developer relies on the inner work of SenderPubKey or manually instanciate Principal, the API changed. commit a254861ae4bfe608734c8c190e4033cfe39f047b Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:17:32 2020 -0800 chore(deps): bump node-fetch from 2.6.0 to 2.6.1 in /e2e/node (#49) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 694849109bc28fea80119388504127e889b36d15 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:17:14 2020 -0800 chore(deps-dev): bump node-fetch from 2.6.0 to 2.6.1 in /packages/agent (#50) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 88af47f79b5e9f3a5e08abbf0ce03b3140357fc0 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Nov 6 15:16:53 2020 -0800 chore(deps-dev): bump node-fetch in /packages/bootstrap (#51) Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1. - [Release notes](https://github.com/bitinn/node-fetch/releases) - [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md) - [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit ba4b082e5795150176f7adad989c3613edceb85f Author: Hans <hans@larsen.online> Date: Thu Nov 5 12:49:59 2020 -0800 test: add ic-ref, ci e2e and lerna support to repo (#47) This refactors the repository to use lerna to manage its monorepo's packages. It also adds the e2e tests from the SDK repo that we previously had to remove. More e2e tests will be added later, but this unblocks working with DER and Identity work as we can now at least run some e2e tests now (instead of mocking the http connection). commit 8baa1cfc227d003c2806df1057b92009159df250 Author: Hans <hans@larsen.online> Date: Tue Nov 3 11:04:36 2020 -0800 feat: use anonymous principal by default in HttpAgent (#46) If the auth transform is missing and the principal is anonymous, use an anonymous auth transform. Otherwise, use the auth transform as normal. # BREAKING CHANGE Before, an auth transform would not be called if it was missing, now it results as an error if the principal is not anonymous. This was because just using the packet as is without putting it in an envelope was a bug and should never be sent to a replica anyway. Now this is explicit. commit 1a714e6c53f5e467112b964b71204b6d2eb545ef Author: Hans <hans@larsen.online> Date: Tue Nov 3 11:04:15 2020 -0800 feat: add a getPrincipal() method to Agents (#40) commit 324e856944adb7f79a2afa2cffbdb6d144b35ab4 Author: Hans <hans@larsen.online> Date: Tue Nov 3 08:49:44 2020 -0800 feat: add the canister ID to the window.ic object (#41) This can be useful to know the canister ID of the current frontend. commit 8680a2cf4994b45c6e00ffb0b5a12b379f5ff919 Author: Hans <hans@larsen.online> Date: Mon Nov 2 21:57:21 2020 -0800 feat: add a cache bust hash at the end of javascript output (#39) commit 9f2ed968b7cc9f4b854af5e23edca19d34124909 Author: Benjamin Goering <benjamin.goering@dfinity.org> Date: Tue Sep 22 15:28:49 2020 -0700 docs: polish docs/npm-publish.md per @hansl feedback (#37) via: pr feedback on dfinity/agent-js#36

hansl requested review from paulyoung, matthewhammer, nomeata and a user September 20, 2019 19:51

hansl force-pushed the hansl/canister-call branch 2 times, most recently from 5893e85 to 270b856 Compare September 20, 2019 20:14

paulyoung suggested changes Sep 20, 2019

View reviewed changes

hansl force-pushed the hansl/canister-call branch 2 times, most recently from bc22970 to 30e4914 Compare September 20, 2019 23:12

hansl requested a review from paulyoung September 20, 2019 23:57

paulyoung suggested changes Sep 23, 2019

View reviewed changes

paulyoung reviewed Sep 24, 2019

View reviewed changes

paulyoung approved these changes Sep 24, 2019

View reviewed changes

hansl and others added 5 commits September 24, 2019 11:32

feat: canister call command

0b14edc

feat: add canister query command

775f36f

refactor: move CanisterId type to lib

58395d2

refactor: create a new clap validators module for argument validation

4eafb10

And move the is_number into it, with is_canister instead to allow for canister ID proper typing.

feat: removed call root command

fa6d0bb

DEPRECATION: please use "cannister call" instead

hansl force-pushed the hansl/canister-call branch from f923b69 to fa6d0bb Compare September 24, 2019 18:32

hansl merged commit b67f56e into master Sep 24, 2019

hansl deleted the hansl/canister-call branch October 10, 2019 21:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Canister query and call commands #39

Canister query and call commands #39

hansl commented Sep 20, 2019

paulyoung Sep 20, 2019

hansl Sep 20, 2019

paulyoung Sep 20, 2019

hansl Sep 20, 2019

paulyoung Sep 21, 2019

hansl Sep 24, 2019

paulyoung Sep 24, 2019

paulyoung Sep 20, 2019

hansl Sep 20, 2019

paulyoung Sep 21, 2019

hansl Sep 24, 2019

paulyoung left a comment

paulyoung Sep 23, 2019

paulyoung Sep 23, 2019

hansl Sep 24, 2019 •

edited

Loading

hansl Sep 24, 2019

paulyoung Sep 23, 2019 •

edited

Loading

hansl Sep 24, 2019

hansl Sep 24, 2019

paulyoung Sep 24, 2019

Canister query and call commands #39

Canister query and call commands #39

Conversation

hansl commented Sep 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulyoung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansl Sep 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulyoung Sep 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansl Sep 24, 2019 •

edited

Loading

paulyoung Sep 23, 2019 •

edited

Loading