dfinity / agent-rs

A collection of libraries and tools for building software around the Internet Computer, in Rust.
https://sdk.dfinity.org/
Apache License 2.0
124 stars 77 forks source link

feat!: add support for fetching root keys automatically #569

Open rupansh opened 4 months ago

rupansh commented 4 months ago

Description

This change introduces a way to fetch root keys automatically on demand. This makes the construction of the agent a bit more ergonomic for local nodes. Minor breakage to the API had to be introduced unfortunately. I also recommend deprecating the fetch_root_keys method.

How Has This Been Tested?

I ran cargo test. Further, this version will also be used by the Yral fronend for testing our changes locally (https://github.com/go-bazzinga/hot-or-not-web-leptos-ssr/blob/rupansh/testcontainers/src/state/canisters.rs#L30)

Checklist:

sa-github-api commented 4 months ago

Dear @rupansh,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA.

If you decide to agree with it, please visit this issue and read the instructions there. Once you have signed it, re-trigger the workflow on this PR to see if your code can be merged.

— The DFINITY Foundation

adamspofford-dfinity commented 4 months ago

Why is this needed vs just calling fetch_root_key immediately after constructing the agent?

rupansh commented 4 months ago

Why is this needed vs just calling fetch_root_key immediately after constructing the agent?

This proposes a more ergonomic API, fetch_root_key is a part of construction of the agent, currently this becomes a 2 part process

  1. You first configure the agent through the builder
  2. You have to further configure the agent through a second call to fetch_root_key.

Other than the ergonomic benefits, the construction of the agent becomes synchronous instead of requiring async. This is useful if you need a global static agent, or you are in an environment where initializing the agent asynchronously might not be trivial (we encountered this while using the agent in leptos for example)

Imagine a global static agent, before every call to the replica, one would have to ensure fetch_root_key is called, pervasing the call throughout the code base.

adamspofford-dfinity commented 4 months ago

Other than the ergonomic benefits, the construction of the agent becomes synchronous instead of requiring async. This is useful if you need a global static agent

Only for lazy-init statics (e.g. LazyLock), yes? Init-once statics (e.g. OnceLock) have no trouble being async. And for an agent which may or may not be pointed at the local replica, doesn't this already lend itself to init-once style due to the replica being used being a runtime parameter?

we encountered this while using the agent in leptos for example

Do you have an example of this?

rupansh commented 4 months ago

Other than the ergonomic benefits, the construction of the agent becomes synchronous instead of requiring async. This is useful if you need a global static agent

Only for lazy-init statics (e.g. LazyLock), yes? Init-once statics (e.g. OnceLock) have no trouble being async. And for an agent which may or may not be pointed at the local replica, doesn't this already lend itself to init-once style due to the replica being used being a runtime parameter?

Indeed

we encountered this while using the agent in leptos for example

Do you have an example of this?

Sure, most of the code is written in a dual-environment style in leptos(i.e targeting both browser and server): We are providing a global agent here: https://github.com/go-bazzinga/hot-or-not-web-leptos-ssr/blob/rupansh/testcontainers/src/app.rs#L65

A way to force async initialization would be by using create_blocking_resource, but these run server side and need the result to be Serializable (which agent is not) Technically I can send the root key server side as a workaround but these are further reduced ergonomics, and introduces latency to the initial load of the website

I understand that this might be too specific to our use case which is why my main emphasis is on the ergonomics rather than better global statics