Hardcode the IC public key into the rust agent, to be used unless fetch_root_key is called.

Add a command-line option to icx to fetch the root key

It should work if using ic-ref as the backend.

Yak shaving:

• icx should be able to upload asset files and directories.

Tests to do:

• regular e2e tests for using as a proxy to connect to
• using an asset canister, downloading http assets

This code in ic-agent/src/agent/agent_error.rs hits an unwrap if a server error does not set a content type:

              HttpErrorPayload {
                  status,
                  content_type,
                  content,
              } => {
                  f.write_fmt(format_args!(
                      "Http Error: status {}, content type {:?}, content: {:?}",
                      status,
                      content_type.as_ref().unwrap(),
                      content
                  ))?;
              }

Upgrading the minor version of ic-types break the build.

Fetch from the status endpoint.

dfx will need to control whether this is necessary (the "for dev instances" part)

I added ic-utils = "0.3.1" to the [dependencies] section of my Cargo.toml file so I could do the following:

use ic_cdk_macros::{query};
use ic_utils::interfaces::http_request::{HttpResponse, HttpRequest};

#[query]
fn http_request(request: HttpRequest) -> HttpResponse {
  ...
}

However, that caused cargo build --target wasm32-unknown-unknown --package <my package> --release to fail.

ic-utils appears to transitively depend on openssl-sys, and apparently the openssl crate doesn't have support for WebAssembly.

I'll probably copy and paste what I need from http_request.rs for now but it would be great if we could depend on a crate from this repo that includes them instead.

I recently had to add SyncCallBuilder.with_value_arg(IDLValue), because with_arg panics:

• IDLValue::ty() calls unreachable() : https://github.com/dfinity/candid/blob/master/rust/candid/src/parser/value.rs#L326
• called from here: https://github.com/dfinity/candid/blob/master/rust/candid/src/ser.rs#L28

It would be nicer if with_arg worked with IDLValue rather than panicking.

Using String in the PrincipalError type is blocking this work as it cannot be used in const functions. This means that we have a new error type which duplicated some errors from PrincipalError.

The fix is to make PrincipalError a type that can be returned by the const functions. There are two variants which use String:

• ExternalError which should be removed entirely. Having a custom holder of string is not very useful in this specific case.
• AbnormalTextualFormat which shows what a principal should look like (in case of formatting errors).

This last one can either be split in multiple error types with better information (like invalid checksum, groups, etc). Or it could contain a stack allocated str. Since principal textual format can only hold base32 chars (+ -s) this should be trivial to make (something like [u8; MAX_LENGTH]). Having fixed length slices though make this type bloat every Result return values, so this should be considered.

However, I need to verify it in the previous Safari browser. The cache has been cleared, so I cannot verify it. As a result, I cannot restore my account, what should I do

#71

#75

call as defined here takes a destination of Canister type and in the body, only grabs destination's canister ID destination.canister_id_().clone()

    /// Forward a call to another canister, including an amount of cycles
    /// from the wallet.
    pub fn call<'canister: 'agent, Out, M: Into<String>>(
        &'canister self,
        destination: &'canister Canister<'canister>,
        method_name: M,
        arg: Argument,
        amount: u64,
    ) -> CallForwarder<'agent, 'canister, Out>
    where
        Out: for<'de> ArgumentDecoder<'de> + Send + Sync,
    {
        CallForwarder {
            wallet: self,
            destination: destination.canister_id_().clone(),
            method_name: method_name.into(),
            amount,
            arg,
            phantom_out: std::marker::PhantomData,
        }
    }

It could take a Principal instead.

I see that icx-proxy has inherited the logic of picking canister ids from hostname, query string, or referer, in that order from the dfx bootstrap command:

But support for query and referer is only needed for local development, right? It might be confusing if this logic is used “in production”.

I suggest to make looking into the query string and referrer optional, and only enable it when icx-proxy is run as part of dfx start.

The action is useful for reviewers to read the rendered documentation, but it doesn't seem to work all the time. This might just be as simple as changing which action it uses.

Once fixed re-enable the workflow.

DNS Aliases should be compared from longest to shortest, as a.b.c should resolve even if we also resolve b.c to something else, if both a passed as aliases.

I saw this in a log produced by @crusso with a very long-running call:

[nix-shell:~/dfx0.6.18/doexp]$ time dfx canister call doexp sumAsync 100
The replica returned an HTTP Error: Http Error: status 400 Bad Request, content type "text/plain; charset=UTF-8", content: Could not convert to a Query or request_status message: InvalidIngressExpiry("Specified ingress_expiry 1610903616250571123ns is less than allowed expiry time 1610903617575674526ns")
real	4m3.068s
user	0m2.761s
sys	0m0.044s

This looks like a bug: Why should the signature on a read ever be “old” (unless clocks are askew or something).

Reading through the code it seems that this is the problem:

    pub async fn call_and_wait<W: Waiter>(&self, mut waiter: W) -> Result<Vec<u8>, AgentError> {
        let request_id = self
            .agent
            .update_raw(
                &self.canister_id,
                self.method_name.as_str(),
                self.arg.as_slice(),
                self.ingress_expiry_datetime,
            )
            .await?;
        waiter.start();

        loop {
            match self
                .agent
                .request_status_raw(&request_id, self.ingress_expiry_datetime)
                .await?
      …

I think there is no reason to pass the self.ingress_expiry_datetime to request_status_raw: Even if the user set a deadline on when the call should be ingressed, the call can always take longer to process, and fetching the status should always use a fresh signature.

Proposed solution: Simply remove the ingress_expiry_datetime parameter from request_status_raw and self.read_state_raw.

Note that the field name ingress_expiry is a wrong name, and it really should be something like “signature_valid_until” (then it makes more sense, right?)

Currently blocked by https://github.com/dfinity-lab/ic-ref/pull/90

Hello, hope all is well. Do we have an example reference server, to run the tests against (other than the IC)?
If so, how would I go about building this locally and running the tests?

Thank you.

When I was testing the callback, I encountered an error in the title, which seemed to indicate that the body in the main thread had been released when the thread was sending data

The identity service team is asking that they use identity.ic0.app as their DNS. This resolves to a specific canister ID. This means that the icx-proxy needs to be able to resolve this DNS to their canister.

Proposed Design

Add a --dns-alias identity.ic0.app:aaaaa-aa command line argument that maps domain names to canister IDs. This would be checked first before using any other resolution logic. Any additional subdomains (in this example, abc.identity.ic0.app) would also match.

This command line argument can be repeated to add more aliases.

A lot of languages we don't support at least have ffi support, so we should have a way to build the Rust Agent and link it with those languages.

Use SoftHSM to test HardwareIdentity against ic-ref.

        loop {
            match self.agent.request_status_raw(&request_id).await? {
…
                RequestStatusResponse::Unknown => (),
                RequestStatusResponse::Received | RequestStatusResponse::Processing => {
                    // The system will return Unknown until the request is accepted
                    // and we generally cannot know how long that will take.
                    // State transitions between Received and Processing may be
                    // instantaneous. Therefore, once we know the request is accepted,
                    // we restart the waiter so the request does not time out.
                    if !request_accepted {
                        waiter
                            .restart()
                            .map_err(|_| AgentError::WaiterRestartError())?;
                        request_accepted = true;
                    }
                }
…
            };

will very aggressively hammer the read endpoint, possibly starving that node. [There should be some delays here.]

Or possibly text/*

Example error that this would affect:

The replica returned an HTTP Error: Http Error: status 502 Bad Gateway,
content type "text/html", 
content: [60, 104, 116, 109, 108, 62, 13, 10, 60, 104, 101, 97, 100,
 62, 60, 116, 105, 116, 108, 101, 62, 53, 48, 50, 32, 66, 97, 100, 32, 
71, 97, 116, 101, 119, 97, 121, 60, 47, 116, 105, 

Now that Principal is no longer heap based, more of it can be made const.

Right now, we parse the subdomain from the left and the first subdomain that parses properly to a canister ID we encounter is used.

This means that a URL like cid1.cid2.ic0.app would be resolved to cid1 but a browser would allow cookies and local storage from cid2 to be read. This is a clear security risk.

Currently this is mitigated by the fact that we only support 1 level of domain names under ic0.app, but it is incidental and probably best to have a proper fix in icx-proxy.

Proposed Design

Add a (repeatable) command line flag --dns-suffix=ic0.app which would be used to validate that a virtual host matches this proxy. If it matches, take the next subdomain and resolve it as a canister ID. In this example, if the user passed aaaaa.ic0.app, the proxy code would try to use aaaaa to resolve the canister ID (in this case an invalid string). The first match in the repeating arguments would be used.

This allows for high customization of the hostname that is used for the icx-proxy, even on custom domains if we ever want to expand support. It also allows for later supporting DNS resolution from canisters.

If this flag is not passed, default it to localhost.

see: #30 (comment)

• query_raw should be private
• QueryBuilder should be added with support for similar functionality to UpdateBuilder

Everything is scriptable so there's no reason we couldn't make a green button to create the release PR then the crates.io release.

If we could get icx to be a full binary to send and sign queries and update calls to the Internet Computer, it would be trivial to build it for targets that aren't accessible by dfx. Also, it would be very lean and perfect for environments that are resource limited (like CI or air-gap machines).

Here's a non-exhaustive list of enhancements

• get rid of the principal conversion stuff (we can move it to a separate tool later)
• update the sign output to produce the same JSON schema as dfx
• update the send function to take the same JSON schema
• add a signed_request_status field to the JSON or something that (on update) signs also a request status 5 minutes after the call is planned to expire.
• add a new argument --status to send in icx.

Function AgentBuilder::with_identity has the following comment:

This is required.

It should not be required. By default, it should use the anonymous user. The spec reads:
https://docs.dfinity.systems/public/#authentication

The fields sender_pubkey and sender_sig must be omitted if and only if the sender field is absent or the anonymous principal.

As mentioned in #201, I was hoping to depend on the http_request interface from ic-utils.

Having worked around that issue I ran into another: Panicked at 'internal error: entered unreachable code' in the candid value parser.

@chenyan-dfinity had some thoughts on how to address this in dfinity/candid#242

If dfx sent out a user-agent header in the HTTP request we could log that, and could then tell you which versions of dfx were being used, so you would have some idea as to how quickly people are updating, when it's safe to turn down a feature that's only used by an old version, and so on.

One example is https://github.com/dfinity/agent-rs/blob/next/ic-utils/src/interfaces/management_canister.rs#L25-L30 which cannot fail unless we refactor the bits below and break backward compatibility.

We've seen reports of the http2 error : protocol error : not a result of an error in our server logs for dfx users and it appears to be coming from the agent-rs dependency h2. See their issue here: hyperium/h2#530

h2 is used by hyper which is used by reqwuest. This issue is to review our dependencies and evaluate if this (relatively minor) bug is sufficient to consider a different request library.

Currently, AgentConfig is a bare struct with public fields implementing Default. This has 2 issues:

1. Any changes in the Config will break existing user programs.
2. The URL field is initialized with an invalid value to force user to specify it. If user makes a mistake, they will only discover it at runtime. We can easily prevent this mistake at compile time by making the fields pub(crate), removing Default and introducing a Builder initialized with required arguments, e.g.

pub struct AgentConfig { pub(crate) url: Url, ... }

impl AgentConfig {
    pub fn url(&self) -> &Url { ... }
    // ...
}

pub struct ConfigBuilder { ... }
impl ConfigBuilder {
    // Arguments of the [new] function are required.
    pub fn new(url: Url) -> Self { ... }
    // ...
    pub fn build(self) -> AgentConfig { ... }
}

To make a query/update call, there are mainly 4 steps:

1. build a request
2. sign it
3. send it
4. handle the response

ic-agent currently only provide public APIs which do the whole process in an all-in-one call, i.e. QueryBuilder::call() and UpdateBuilder::call().

Recently, the need of separate stepped APIs is rising. In dfx and icx, we added support for the sign/send separation which make it possible to sign the request on an air-gapped machine. Without stepped APIs, we cannot handle the response properly (detail in dfinity/sdk#1676). Also, the stepped APIs will make it easy to inspect the intermediate stage while making a request.

Take Query for example, we would need some extra APIs:

// serde to support sign/send separation
#[derive(Deserialize, Serialize)]
pub struct QueryRequest {
  // info of a query are included, not the cbor encoded bytes 
}

impl QueryBuilder {
  pub fn build(self) -> QueryRequest;

  // we may retain call() for compatibility 
}

impl QueryRequest {
  // argument may be an `Identity`
  pub fn sign() -> Self;
  // return type is the same as QueryBuilder::call()
  pub fn send() -> Result<Vec<u8>, AgentError>;
}

We would need reorganize the private methods of Agent to support above APIs.

ToString allows for types implementing Display to be used, which is just wrong (e.g. a principal could be passed, or a bool).

AgentError prints e.g. the following

The replica returned an HTTP Error: Http Error: status 403 Forbidden, content type "", content: [82, 101, 113, 117, 101, 115, 116, 101, 100, 32, 99, 97, 110, 105, 115, 116, 101, 114, 32, 100, 111, 101, 115, 110, 39, 116, 32, 104, 97, 118, 101, 32, 101, 110, 111, 117, 103, 104, 32, 99, 121, 99, 108, 101, 115]

instead of a human-readable string: "Requested canister doesn't have enough cycles"

Hey!

I just had a pretty hard time figuring out how to use https://agent-rust.netlify.app/ic_agent/agent/struct.updatebuilder#method.with_arg

Because the function is called with_arg, I assumed I had to call it multiple times in the builder pattern to continue adding args. Eventually I tried adding multiple args into with_arg, and it finally worked.

Consider changing the name to with_args?

1. Deployed canister that returns some string when call from DFX.
2. Created rust snippet using ic_agent .

Problem : When calling canister method from the rust code, it throws following error :

error response:ReplicaError {
    reject_code: 5,
    reject_message: "IC0503: Canister qvhpv-4qaaa-aaaaa-aaagq-cai trapped explicitly: Custom(Cannot parse header \n\nCaused by:\n    binary parser error: io error)",
} 

I am sharing snipped and canister
Github link

I think there is some issue with ic_agent ...
Hoping for some quick response from Dfinity team.

The UpdateBuilder::expire_after method sets ingress_expiry_datetime to SystemTime::now() plus a duration offset. https://github.com/dfinity/agent-rs/blob/next/ic-agent/src/agent/mod.rs#L524

Typical usage is something like this:

agent
            .update(&canister_id, CREATE_CHUNK)
            .with_arg(&args)
            .expire_after(timeout)
            .call_and_wait(waiter_with_timeout(timeout))
            .await

This means there can be some delay in between when the absolute expiry is calculated and when the update is actually executed. This length of this delay would be directly related to the number of futures submitted.

It may be that there is no solution at this level, since there are just more await calls deeper in the call stack. Or it may be that moving the calculation into update_raw() (to the point where we build the CallRequest) would in practice help in the case where a number of agent.call_and_wait() futures are awaited all at once.

This new field is part of a call (with probably a default).

• We will need to break update_raw(), which there are two options:

  • Remove update_raw() from our public API, which I support (people should use the builder), or
  • Add an additional argument of type metadata which we can expand in the future and has a sensible Default implementation

• We should add 2 methods to UpdateBuilder:

  • expire_when(time: SystemTime) which takes a system time in the current system's representation and apply transforms to it if needed, and
  • valid_until(duration: Duration)which calculates the ttl from the moment the call is made.

If none of these methods are called, we should use a sensible default (ie. 5 minutes in the future) when the call is made, and if both are called, the last call should be used.

There should not be any mention of u64 (or any number) in those fields. Calculations should not be left to the developer.

The documentation at https://agent-rust.netlify.app/ic_utils/interfaces/management_canister/struct.managementcanister show only 2 functions: create and set_agent.

But the "interesting" functions of the management canister are not directly inside ManagementCanister. Instead, there are in Canister<'agent, ManagementCanister>. For instance, this is where create_canister and install_code is.

I cannot find the documentation for those without looking at the code. Purely using the doc, I really could not understand how I can use the ManagementCanister interface.

We should make sure that the doc of ManagementCanister links to wherever the "real" doc of the interface is.

I tested today ic-agent 0.10.0 and when I tried to create a transport via ReqwestHttpReplicaV2Transport::create(url) I got the error

'Could not create HTTP client.: reqwest::Error { kind: Builder, source: "Unknown TLS backend passed to `use_preconfigured_tls`" }'

I found out that the reason is that ic-agent depends on reqwest 0.11 and rustls 0.19.1 but the newer reqwest (0.11.7) uses rustls 0.20.1. ic-agent uses the following code to instantiate a transport

reqwest::Client::builder().use_preconfigured_tls(tls_config)

and the method use_preconfigured_tls docs says:

This is an advanced option, and can be somewhat brittle. Usage requires keeping the
preconfigured TLS argument version in sync with reqwest, since version mismatches
will result in an “unknown” TLS backend.

A fix for this is to enforce the version of reqwest to be =0.11.6 (I tested this locally). Another solution is to bump rustls but the newer version has a different API.

I would also suggest to either not use use_preconfigured_tls or always fix the versions of reqwest and rustls to avoid having this issue again.

Currently the only way to type a result of an update call is to use call_and_wait(). By creating a typed version users can wait on the request id and get the proper type if they know it in advance (or error on deserialization if it doesn't match).